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 Headers as Attributes for Otel traces #673

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

panizzag
Copy link

GUS Associated: W-12558102

This commit adds the possibility that the headers processed by the HTTP listener and by the HTTP requester are added as attributes of the spans initialized during the Otel implementation. This new feature takes into account the option of omitting headers that you do not want to register by using a property that is configured at the config element level of both components. An example of use is as follows:

  • HTTP Listener

<http:listener-config name="HTTP_Listener_config" doc:name="HTTP Listener config" doc:id="f83efe45-479a-498f-adc1-8848f33ed380" skipHeadersOnTracing="client_id, client_secret">

  • HTTP Requester

<http:request-config name="HTTP_Request_configuration" doc:name="HTTP Request configuration" doc:id="3a18fe34-e53b-4ca4-80e2-229cb386f1bb" preserveHeadersCase="true" skipHeadersOnTracing="client_id, client_secret">

  • Example cUrl request
    curl --location --request GET 'localhost:8081/test' \ --header 'X-Example-Header: Test' \ --header 'client_id: thisIsAnExampleClientID' \ --header 'client_secret: thisIsAnExampleClientSecret'

The final result is the following:

  • Before the change - Attributes registered for the example call

    • Listener:
    image
    • Requester:
    image
  • After the change - Attributes registered for the example call

    • Listener
      image

    • Requester
      image

@panizzag panizzag requested a review from a team as a code owner February 16, 2023 18:42
pom.xml Outdated Show resolved Hide resolved
@@ -41,16 +46,24 @@ public class HttpListenerCurrentSpanCustomizer extends HttpCurrentSpanCustomizer
private final String host;
private final int port;

private HttpListenerCurrentSpanCustomizer(HttpRequestAttributes attributes, String host, int port) {
//W-12558102
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the comment for the feature.

Copy link
Author

Choose a reason for hiding this comment

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

Done. Should I remove all the other similar comments?

@fsgonz
Copy link
Contributor

fsgonz commented Feb 16, 2023

Thanks very much for the PR, @panizzag!
I have added some cosmetic request for changes for the moment (just in case you have time to add them)
Can you verify if you some unit tests for that. We will be working on adding some integration tests.
Probably we will have to discuss a little bit the new feature with the team but it clear and straightforward.

@panizzag
Copy link
Author

panizzag commented Feb 16, 2023

Thanks, @fsgonz for the review and suggestions. I have expanded the existing test case to include the new features. I'd appreciate it if the team can give it a new look as there might be coding guidelines and styles that I'm not used to. Thanks

@fsgonz
Copy link
Contributor

fsgonz commented Feb 16, 2023

Thanks very much. We'll also have to verify if we follow the semantics conventions
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#http-request-and-response-headers
And if it is enabled by default. Thanks again

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.

3 participants