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

Log errors that occur during /explain #134

Merged
merged 2 commits into from
Nov 3, 2023
Merged

Conversation

plcplc
Copy link
Contributor

@plcplc plcplc commented Nov 2, 2023

What

Make an OpenTelemetry log event when the queries to /explain encounter an error.

How

OpenTelemetry log events are simply events with the field meta.signal_type = "log", and otherwise adhering to the semantic conventions of logs, see https://opentelemetry.io/docs/specs/otel/logs/data-model/.

Similarly, we set error = true, which both Honeycomb and Jaeger recognizes in the UI.

The tracing::event!(..) macro captures the place in the code where it was called. Therefore I have opted to not try and abstract over making error-log events, as that information becomes just noise otherwise.

The body attribute of the event/log is just the Display'd Error message. Depending on the future uses we want for these log messages we will probably amend them with more attributes/ more information.

In Jaeger:
image

In Honeycomb:
image

@SamirTalwar
Copy link
Contributor

Are you planning on making the same change to query.rs?

@plcplc
Copy link
Contributor Author

plcplc commented Nov 3, 2023

Are you planning on making the same change to query.rs?

Yes, and potentially other places as described in the issue. I just wanted to have one thing in before I started duplicating it.

@plcplc plcplc added this pull request to the merge queue Nov 3, 2023
Merged via the queue into main with commit 32ac9f8 Nov 3, 2023
26 checks passed
@plcplc plcplc deleted the plc/issues/NDAT-966-explain branch November 3, 2023 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants