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

add otel.instrumentation.http.server.add-server-attributes #6520

Closed

Conversation

zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Jun 13, 2024

Background:

  • exploring different options for making it easier to opt-in to server.address and server.port for http.server.request.duration
  • these 2 attributes are special in that they allow the creation of a service graph - for services that don't send spans
  • the best opt-in mechanism should be implemented in all languages

Alternatives:

@zeitlinger zeitlinger self-assigned this Jun 13, 2024
Copy link

codecov bot commented Jun 13, 2024

Codecov Report

Attention: Patch coverage is 0% with 17 lines in your changes missing coverage. Please review.

Project coverage is 90.57%. Comparing base (a686280) to head (f3f4142).

Files Patch % Lines
...nfigure/AutoConfiguredOpenTelemetrySdkBuilder.java 0.00% 16 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6520      +/-   ##
============================================
- Coverage     90.65%   90.57%   -0.09%     
  Complexity     6227     6227              
============================================
  Files           678      678              
  Lines         18659    18676      +17     
  Branches       1842     1843       +1     
============================================
  Hits          16915    16915              
- Misses         1187     1203      +16     
- Partials        557      558       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@trask
Copy link
Member

trask commented Jun 13, 2024

can you explain what problem this is solving? thanks

@@ -448,6 +453,25 @@ public AutoConfiguredOpenTelemetrySdk build() {

if (sdkEnabled) {
SdkMeterProviderBuilder meterProviderBuilder = SdkMeterProvider.builder();
if (config.getBoolean("otel.instrumentation.http.server.add-server-attributes", false)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this belongs in our core SDK autoconfigure. This looks to be an instrumentation-specific option. Let's talk about the right place for this configuration in a SIG meeting.

@jack-berg
Copy link
Member

We've added the first instrumentation configuration options in file configuration here: open-telemetry/opentelemetry-configuration#91

Separately, this does not belong as a core SDK configuration.

@jkwatson
Copy link
Contributor

@zeitlinger is there any reason to keep this open, given that both maintainers do not believe this is appropriate to build into the core repo?

@zeitlinger
Copy link
Member Author

No

@zeitlinger zeitlinger closed this Jun 28, 2024
@zeitlinger zeitlinger deleted the add-server-attributes branch July 1, 2024 09:53
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.

4 participants