Consistent application of exporter customizers when otel.{signal}.exporter=none #7017
Conversation
| @@ -78,7 +78,6 @@ testing { | |||
| environment("OTEL_PROPAGATORS", "tracecontext,baggage,b3,b3multi,jaeger,ottrace,test") | |||
| environment("OTEL_EXPORTER_OTLP_HEADERS", "cat=meow,dog=bark") | |||
| environment("OTEL_EXPORTER_OTLP_TIMEOUT", "5000") | |||
| environment("OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", "2") | |||
There was a problem hiding this comment.
This was interfering with the changes I was making to FullConfigTest. Setting span limits is well tested elsewhere, and we should limit the integration-style FullConfigTest to concepts that really require everything to be stood up in an end-to-end fashion.
...ure/src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7017 +/- ##
============================================
+ Coverage 89.95% 89.98% +0.02%
+ Complexity 6636 6635 -1
============================================
Files 745 745
Lines 20010 20000 -10
Branches 1962 1960 -2
============================================
- Hits 18000 17996 -4
+ Misses 1415 1408 -7
- Partials 595 596 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks for noticing and fixing the inconsistency! It would definitely be surprising to someone somewhere if they specified otel.{signal}.exporter=none and some customizer injected an exporter anyway. And I agree with your assessment that you could argue for/against this approach, but I think this way is the most practical/sensical.
While working on something else, I noticed a discrepancy between how autoconfigure handles exporter customization via SPI when
otel.metrics.exporter=none, compared tootel.traces.exporter=noneandotel.logs.exporter=none.For traces and logs, when exporter is set to
none, we still invoke any respective exporter customizers registered viaAutoConfigurationCustomizer#addSpanExporterCustomizer,AutoConfigurationCustomizer#addLogRecordExporterCustomizer. For metrics, we do not apply such aAutoConfigurationCustomizer#addMetricExporterCustomizerwhen exporter is set tonone.This PR standardizes on the the metrics approach of not applying exporter customizers when exporter is set to
none.While you can make arguments for either approach, the metrics approach is more correct / defensible since its less surprising. I don't think many people would intuit that exporter customizers have a chance to intervene when the user intends to set the exporter to none.