Conversation
|
@meryl-c, please consider creating a changeset entry in |
|
thanks for getting started on looking into this. i'm going to mark this as a draft rather than being "ready for review". It appears like it needs some work before it's ready to me, but I could be wrong. If there was something specific that you think is ready for review, could you comment back what it is? 🙇 |
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 12 changed, 0 removedBuild ID: ebfe35dfcbe7e461343e0b34 URL: https://www.apollographql.com/docs/deploy-preview/ebfe35dfcbe7e461343e0b34 |
|
@abernix thanks! I didn't even notice it wasn't a draft 😅 |
| opentelemetry-otlp = { version = "0.17.0", default-features = false, features = [ | ||
| opentelemetry-http = "0.30.0" | ||
| opentelemetry-jaeger-propagator = "0.30.0" | ||
| opentelemetry-otlp = { version = "0.30.0", default-features = false, features = [ |
There was a problem hiding this comment.
The tonic dependency in apollo-router is just to create instances of structures for use with opentelemetry-otlp. In new versions of opentelemetry-otlp, everything we need is reexported here: https://docs.rs/opentelemetry-otlp/0.30.0/opentelemetry_otlp/tonic_types/index.html
So we can remove our tonic dependency and replace our tonic imports with use opentelemetry_otlp::tonic_types::{etc}.
There was a problem hiding this comment.
@goto-bus-stop @BrynCooke it doesn't export MetadataValue, which we do use:
There was a problem hiding this comment.
I ended up upgrading tonic to 0.13.1 to fix another issue. if there's a workaround to using MetadataValue, I'm happy to do that instead and remove tonic
…kTracerProvider`
…ove usage of `set_error_handler` (removed in otel rust 0.27.0)
…n issue because `FilterMeterProvider` extends `MeterProvider`, which requires a `meter_with_scope` method
|
currently stuck because when I was removing |
…ges should be combined into one)
…reaking change. There is a later one that gets rid of `MetricError` entirely
… telemetry/otlp.rs
…d otlp to allowed_features configs
Also moves with_filter logic directly into fmt_layer to avoid tracing reload bug
| fn strip_noisy_otel_shutdown_logs(s: &str) -> String { | ||
| let had_trailing_newline = s.ends_with('\n'); | ||
| let filtered = s | ||
| .lines() | ||
| .filter(|line| if line.trim().contains(r#""name":"MeterProvider.Drop""#) { false } else { true } ) | ||
| .collect::<Vec<_>>() | ||
| .join("\n"); | ||
| if had_trailing_newline && !filtered.is_empty() { | ||
| format!("{filtered}\n") | ||
| } else { | ||
| filtered | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This probably isn't the best long term. I wonder if this is actually due the ReemitOtelLayer not going through the regular tracing layers? Otherwise perhaps it would be muted?
6 participants
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
A lot of (if not most) features benefit from built-in observability and
debug-level logs. Please read this guidance on metrics best-practices. ↩Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩