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

[http.server.request.duration] New origin.name attribute #1336

Open
alexa-gt opened this issue Aug 15, 2024 · 1 comment
Open

[http.server.request.duration] New origin.name attribute #1336

alexa-gt opened this issue Aug 15, 2024 · 1 comment
Assignees
Labels
enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve triage:needs-triage

Comments

@alexa-gt
Copy link

alexa-gt commented Aug 15, 2024

Area(s)

http.server.request.duration

Is your change request related to a problem? Please describe.

Background

I was using instrumentation-http with the sdk-node autoinstrumentation and I could not find seem to find an option for me to be able to specify that a custom attribute I added to a span in requestHook or headersToSpanAttributes.

The use case for this would be to identify traffic flow per client of an internal microservice application.

flowchart LR
    B --> A
    C -->|HTTP/GRPC + 'X-Origin' header/metadata| A
    D --> A
Loading

This would be highly valuable in detecting spikes from some upstream service or checking if some % of problematic traffic comes from some origin service as the immediate parent.

Describe the solution you'd like

Proposal

Add an origin.name attribute to be able to segregate inbound metrics on A by direct clients. The field value should correspond to service.name of the upstream, in the above case, B, C or D.

Describe alternatives you've considered

No response

Additional context

No response

@alexa-gt alexa-gt added enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve triage:needs-triage labels Aug 15, 2024
@lmolkova
Copy link
Contributor

lmolkova commented Aug 16, 2024

[UPDATE] Please take a look at the existing peer.service attribute -

| `peer.service` | string | The [`service.name`](/docs/resource/README.md#service) of the remote service. SHOULD be equal to the actual `service.name` resource attribute of the remote service if any. | `AuthTokenCache` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |

I believe it fits for the use-case perfectly.

Propagating service name in the headers looks like a very custom solution.

If the intention is to make all HTTP server instrumentations enrich metrics with it, there need to be a common general-purpose solution that defines a propagation mechanism. It might be a good idea to outline the whole solution and share it as an OTEP or at least start a discussion in the specification repo.

Keep in mind that adding new attributes to metrics is a breaking change and should be handled with caution - #722.

One more thing to keep in mind is that http client metrics coming from upstream services already have all the information, so, if you have access to their telemetry, it could be easier to analyze it rather than propagate service name downstream.

If the intention is to have custom propagation solution and use custom server instrumentation, then please share some context why common standard attribute is necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve triage:needs-triage
Projects
None yet
Development

No branches or pull requests

3 participants