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 Instrumentation (metric) for net/http #1336

Closed
10 tasks
victoraugustolls opened this issue Oct 8, 2021 · 6 comments · Fixed by #4707 · May be fixed by #3769
Closed
10 tasks

Add Instrumentation (metric) for net/http #1336

victoraugustolls opened this issue Oct 8, 2021 · 6 comments · Fixed by #4707 · May be fixed by #3769
Labels
area: instrumentation Related to an instrumentation package enhancement New feature or request instrumentation: otelhttp

Comments

@victoraugustolls
Copy link

victoraugustolls commented Oct 8, 2021

Background

Package Link: net/http

stdlib http client.

Proposed Solution

Collect http.client.duration metric from the stdlib http client. Currently I'm using a custom RoundTripper that calls the otelhttp tracing RoundTripper, and I think it would be nice if this was inside the contrib. If agreed, I can open a PR for it!

I see something similar was done in #427 , but it was unfortunately closed! That makes me think that this might not be desired? This is why I'm asking in advance. Thanks!

Metrics

Instruments

  • http.client.duration: measure the duration of the outbound HTTP request
    • type: Histogram
    • unit: milliseconds
    • description: measure the duration of the outbound HTTP request

Prior Art

Tasks

  • Code complete:
    • Comprehensive unit tests.
    • End-to-end integration tests.
    • Tests all passing.
    • Instrumentation functionality verified.
  • Documented
  • Examples
    • Dockerfile file to build example application.
    • docker-compose.yml to run example in a docker environment to demonstrate instrumentation.
@victoraugustolls victoraugustolls added area: instrumentation Related to an instrumentation package enhancement New feature or request release:after-ga labels Oct 8, 2021
@victoraugustolls victoraugustolls changed the title Add Instrumentation for <package-name> Add Instrumentation (metric) for net/nttp Oct 8, 2021
@victoraugustolls victoraugustolls changed the title Add Instrumentation (metric) for net/nttp Add Instrumentation (metric) for net/http Oct 8, 2021
@MrAlias
Copy link
Contributor

MrAlias commented Oct 14, 2021

In accordance with the new instrumentation policy hosting guidelines I'm closing this.

@MrAlias MrAlias closed this as completed Oct 14, 2021
@vmihailenco
Copy link
Contributor

Are we supposed to fork otelhttp to add metrics?

@victoraugustolls
Copy link
Author

Sorry @MrAlias , but I don't quite follow. Being a standard library instrumentation, isn't this repo the right place to put it? I didn't find a good reason for not contributing for it here. Sorry if I missed something!

@MrAlias
Copy link
Contributor

MrAlias commented Oct 25, 2021

Ah, yeah. Closing was a mistake given we already host this instrumentation.

@MrAlias MrAlias reopened this Oct 25, 2021
@victoraugustolls
Copy link
Author

victoraugustolls commented Oct 26, 2021

@MrAlias No worries! So I'm guessing this is a ok for developing this? If that's the case, I can work on it!

@tonglil
Copy link
Contributor

tonglil commented Nov 19, 2021

Personally I feel like that kind of information is critical to the instrumentation. As far as I can tell, the otelhttp instrumentation is mostly barebones at the moment and only records HTTP GET/POST/etc, like without parameterized http.url and http.target attributes that I feel like are needed.

Would love to see this data @victoraugustolls!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: instrumentation Related to an instrumentation package enhancement New feature or request instrumentation: otelhttp
Projects
None yet
4 participants