Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display sync error messages from connectors (use AirbyteTraceMessage to populate failure reason) #12161

Closed
Tracked by #12118
lmossman opened this issue Apr 19, 2022 · 5 comments

Comments

@lmossman
Copy link
Contributor

lmossman commented Apr 19, 2022

What

See the tech spec for full context on this issue: https://docs.google.com/document/d/1nDVU-6_lDHkNVHPFHwfozaEoGl9hmV42RaAbv__6Bqo/edit

We would like to offer the ability for connectors to send the platform "display" messages that will be shown to the user, as described in the above tech spec. We will be going with Proposal 2.

This ticket only focuses on changing the protocol and setting the "failure reason" for a sync attempt to one of these messages if they exist for that attempt.

How

This ticket is composed of two changes, as described in the tech spec linked above:

  • Add the AirbyteDisplayMessage object to the airbyte protocol, with a matching AirbyteMessage type (e.g. DISPLAY)
  • Make the Airbyte UI use the first ERROR AirbyteDisplayMessage as the failure reason of the attempt (if none, default to current behavior)
    • This will involve modifying the DefaultReplicationWorker / DefaultAirbyteStreamFactory to look for DISPLAY AirbyteMessages, and if it finds any with level: ERROR, set add those to the failure reasons of the attempt.
    • Note: as described in the implementation notes of the tech spec, the UI currently selects the first failureReason of the attempt to show in the UI. Therefore, the change here will need to ensure that the first ERROR AirbyteDisplayMessage comes first in the attempt's failureReasons list, so that the UI selects that one to use (alternatively, the UI code could be changed to use a different selection strategy)
@evantahler
Copy link
Contributor

evantahler commented Apr 25, 2022

Some notes since this story was initially written:

  • A few extra fields have been added to the AirbyteDisplayMessage spec to better enable reporting and error correlation: internal_message, stack_trace, failure_type, and log_timestamp.
  • As noted above, this additional information should be to fill out the sync attempt's failure reasons - and now we have data for most of those fields.

@evantahler
Copy link
Contributor

#12322 is the epic we can use to track the work to add AirbyteDisplayMessage to the CDK and connectors. This replaces the current behavior of logging and throwring when something goes wrong.

@evantahler
Copy link
Contributor

Noting a spec update - it's AirbyteTraceMessage now!

@evantahler evantahler changed the title Display sync error messages from connectors Display sync error messages from connectors (add AirbyteTraceMessage) May 2, 2022
@lmossman lmossman changed the title Display sync error messages from connectors (add AirbyteTraceMessage) Display sync error messages from connectors (use AirbyteTraceMessage to populate failure reason) May 2, 2022
@sherifnada
Copy link
Contributor

@alovew did this PR #12619 close this issue?

@alovew
Copy link
Contributor

alovew commented May 17, 2022

@sherifnada yep!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants