[KS-368] keystone/workflow engine logging #13563
Conversation
| @@ -15,13 +15,15 @@ import ( | |||
| type LocalTargetCapability struct { | |||
| lggr logger.Logger | |||
| capabilities.TargetCapability | |||
| peerID p2ptypes.PeerID | |||
| don capabilities.DON | |||
| capabilityID string | |||
There was a problem hiding this comment.
Added this so we can log the capability ID that is being wrapped. I didn't want to waste a grpc call on Info when we already have this information
| @@ -350,7 +350,7 @@ func NewApplication(opts ApplicationOpts) (Application, error) { | |||
| jobORM = job.NewORM(opts.DS, pipelineORM, bridgeORM, keyStore, globalLogger) | |||
| txmORM = txmgr.NewTxStore(opts.DS, globalLogger) | |||
| streamRegistry = streams.NewRegistry(globalLogger, pipelineRunner) | |||
| workflowORM = workflowstore.NewDBStore(opts.DS, clockwork.NewRealClock()) | |||
| workflowORM = workflowstore.NewDBStore(opts.DS, globalLogger, clockwork.NewRealClock()) | |||
There was a problem hiding this comment.
This adds a logger to our ORM, i found it pretty useful when debugging failing tests
| @@ -78,6 +80,25 @@ type testHooks struct { | |||
| executionFinished chan string | |||
| } | |||
|
|
|||
| func newTestDBStore(t *testing.T, clock clockwork.Clock) store.Store { | |||
There was a problem hiding this comment.
I added this so we can see a workflow id field in test logs, it was just empty before which made it awkward to read test logs. When the wfid was empty, it would pass the fk constraint, so i had to add this function to make sure that the constraint stays valid with a non empty workflow id
HenryNguyen5
force-pushed
the
keystone/workflow_engine_logging
branch
from
89fe331 to
3e92fc6
Compare
HenryNguyen5
force-pushed
the
keystone/workflow_engine_logging
branch
from
360fa4b to
cef3bb2
Compare
|
HenryNguyen5
changed the title
I ended up creating custom error structs at the workflow engine level, so that we're able to resurface lower level errors as wrapped ones which have fields such as execution id, workflow id, and capability id.
If i didnt do this, there's a mismatch in how we log errors. There are functions that we have which return errors rather than log them. This results in errors which store the context as plaintext inside the error message itself, rather than having them as separate fields.
Ex
Without custom errors you'd get a log message like:
capability id: [email protected] could not find capability in registry..With custom errors, we can log them as:
could not find capability in registry {"capabilityID": "[email protected]"}.