JobErrorReporter: Report connector_command tag for errors encountered during sync #15925
Conversation
github-actions
bot
added
area/platform
pedroslopez
changed the title
pedroslopez
changed the title
| SPEC("spec"), | ||
| CHECK("check"), | ||
| DISCOVER("discover"), | ||
| WRITE("write"), | ||
| READ("read"); |
There was a problem hiding this comment.
Setting a string value for these to keep reporting the existing tags the same way to sentry (check, spec, discover)
There was a problem hiding this comment.
Could add another command for normalize and set that appropriately for the normalizationFailure cases.
There was a problem hiding this comment.
I intended connector_command to map directly to commands that are run against the connectors according to the protocol in order to differentiate more between failures that have an origin of Source or Destination - I think in the case of normalization since the FailureOrigin is always Normalization this shouldn't be necessary (and normalize isn't a real connector command)
| Map.of( | ||
| JOB_ID_KEY, jobContext.jobId().toString(), | ||
| CONNECTOR_COMMAND_META_KEY, "check")); |
There was a problem hiding this comment.
Connector command will now be already set on the FailureReason as metadata, so don't need to set it here when reporting the error
| Map.entry(WORKSPACE_ID_META_KEY, workspaceId.toString()), | ||
| Map.entry(WORKSPACE_URL_META_KEY, workspaceUrl)); |
There was a problem hiding this comment.
This wasn't in scope on the original issue, but added this here quickly since we're not getting any urls for synchronous job errors (due to them not being encountered in the context of a connection)
| SPEC("spec"), | ||
| CHECK("check"), | ||
| DISCOVER("discover"), | ||
| WRITE("write"), | ||
| READ("read"); |
There was a problem hiding this comment.
Could add another command for normalize and set that appropriately for the normalizationFailure cases.
What
fixes #15680
We had been reporting the
connector_commandtag for synchronous jobs (check, spec, discover) but not for errors encountered during the sync process (check (before sync), read, write)How
Moves the connector command tag to being set as metadata on the FailureReason instead of directly when reporting to sentry. This is because we're reporting all
syncerrors from a single spot where we didn't have info whether this came from check-before-sync, reading or writing.The FailureHelper is now in charge of setting this metadata, just like it had been doing with
from_trace_message. The JobErrorReporter now grabs this information from the failure reason. I also added some more testing around the FailureHelper since it was a bit lacking.While I was in here, I also added the
workspace_urlas a tag since the synchronous commands did not run in the context of a connection, so there was no url being set for them.