-
Notifications
You must be signed in to change notification settings - Fork 821
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.target and http.url attributes not set in metrics attributes #3683
Comments
Hi @mkrudele, thanks for reaching out. And thank you for providing the reproducer, that is very helpful 🙂
It looks like the route is never set in this instrumentation, it is just taken over from any other instrumentation that adds it (such as Could you check if adding |
@pichlermarc I tried adding @hermogenes since you implemented te feature, would you shed the lights on how is it supposed to work? Our need is pretty simple. We'd like to have the attribute
but now way, the |
The http route is something only the HTTP server internal knows - it's likely a pattern applied to the received path. The HTTP client can't know this detail. HTTP client sends the see otel http spec for more details. |
@Flarna Thanks for the link to the specs. That's more clear. I understand the point about http client. Still have a question about whether it is possible with the |
Looking into the code it seems the So even if you add
If you redefine I think a more generic approach would be to enhance the hooks (or add new ones) to allow setting attributes on metrics. |
Continued investigation:
This looks like it is the default exporter bubbling up to the I'll open a PR so that no logger is instantiated when the env var is unset. 🙂 In the meantime, you can remedy this error log by either of the following options:
|
@mkrudele Currently, we only add the attributes from the metrics HTTP semantic conventions spec, and we drop any others that are present on the span as to avoid high-cardinality metrics. As @Flarna said, we could enhance the hooks or add new ones to allow users to add their own attributes. Feel free to open a feature-request issue for this. 🙂 As it looks like this issue is a feature request (adding/enhancing the hooks) and a bug report (INFO logger instantiated by default), I'll tag this as priority 3 ( |
What happened?
Steps to Reproduce
Unzip the code.zip to get the code to reproduce the problem.
code.zip
Run one or more
curl -X GET "localhost:6900/proxy_call"
from a terminal.You should see from the simple-express terminal something like:
First problem:
Is this an issue?
That was not happening with versions
0.35.1
of the packages.Second problem:
I was expecting to see an attribute http_target (or http_route or whatever) in the ghost_http_server_duration metric, and the attribute http_url (or equivalent name) in the ghost_http_client_duration metric, as described in this issue #3553 (which should be part of the
0.36.0
version).However, nothing changed since the previous version
0.35.1
. To prove just point your browser to http://localhost:9464/metrics. You should get this:Did I miss something? Is there any config step to follow to enable such attributes in the HTTP metrics?
Expected Result
Expected target path attribute to incoming and outcoming http requests as requested with this issue #3553.
Actual Result
Not getting target path attribute in incoming and outcoming http requests.
Additional Details
OpenTelemetry Setup Code
package.json
Look at the file package.json in code.zip.
Relevant log output
The text was updated successfully, but these errors were encountered: