-
Notifications
You must be signed in to change notification settings - Fork 873
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
Stabilize HTTP headers capturing configuration property names #4459
Stabilize HTTP headers capturing configuration property names #4459
Conversation
| `otel.instrumentation.common.experimental.capture-http-headers.server.response` | `OTEL_INSTRUMENTATION_COMMON_EXPERIMENTAL_CAPTURE_HTTP_HEADERS_SERVER_RESPONSE` | A comma-separated list of HTTP header names. HTTP server instrumentations will capture HTTP response header values for all configured header names. | ||
| System property | Environment variable | Description | | ||
| ----------------------------------------------------------- | ----------------------------------------------------------- | ----------- | | ||
| `otel.instrumentation.http.capture-headers.client.request` | `OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_CLIENT_REQUEST` | A comma-separated list of HTTP header names. HTTP client instrumentations will capture HTTP request header values for all configured header names. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on Thu SIG last week - I replaced the common
part with http
since this property applies to all instrumentations that implement the HTTP semantic convention
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need instrumentation
part here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm in favor of keeping it. If you remove it, you'll be left with otel.http.capture-headers.client.request
which is a bit vague - what does http
refer to? HTTP exporters? Instrumentations? Is this something supported by the SDK, or the javaagent?
| `otel.instrumentation.common.experimental.capture-http-headers.server.response` | `OTEL_INSTRUMENTATION_COMMON_EXPERIMENTAL_CAPTURE_HTTP_HEADERS_SERVER_RESPONSE` | A comma-separated list of HTTP header names. HTTP server instrumentations will capture HTTP response header values for all configured header names. | ||
| System property | Environment variable | Description | | ||
| ----------------------------------------------------------- | ----------------------------------------------------------- | ----------- | | ||
| `otel.instrumentation.http.capture-headers.client.request` | `OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_CLIENT_REQUEST` | A comma-separated list of HTTP header names. HTTP client instrumentations will capture HTTP request header values for all configured header names. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need instrumentation
part here?
...rc/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/CapturedHttpHeaders.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/CapturedHttpHeaders.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
btw, I don't think it falls under our stability guarantee yet, even without the "experimental" word: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/VERSIONING.md#compatibility-requirements. I added to Tue SIG agenda to discuss env var stability.
…elemetry#4459) * Stabilize HTTP headers capturing configuration property names * code review comments
No description provided.