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

url.scheme tag is emitted as part of http.client.request.duration metric that does not match the OTel spec #92077

Closed
vishweshbankwar opened this issue Sep 14, 2023 · 5 comments
Labels
area-System.Net.Http needs-author-action An issue or pull request that requires more info or actions from the author.
Milestone

Comments

@vishweshbankwar
Copy link
Contributor

Description

url.scheme is added by default on http.client.request.duration metric from System.Net.Http. However, it is not part of the required/recommended attributes from the spec.

v1.21
https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md#metric-httpclientduration

Main
https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md#metric-httpclientrequestduration

Reproduction Steps

Test on .NET8.0 RC1 using OTel SDK

Configure SDK

Sdk.CreateMeterProviderBuilder()
.AddMeter("System.Net.Http")
.Build()

Expected behavior

url.scheme is not included by default.

Actual behavior

url.scheme tag is added by default on http.client.request.duration metric

Sample output from sdk

Export http.client.request.duration, The duration of outbound HTTP requests., Unit: s, Meter: System.Net.Http
(2023-09-14T18:44:41.6943865Z, 2023-09-14T18:44:48.7866188Z] http.request.method: GET http.response.status_code: 302 network.protocol.version: 1.1 server.address: www.bing.com url.scheme: http Histogram

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 14, 2023
@ghost
Copy link

ghost commented Sep 14, 2023

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

url.scheme is added by default on http.client.request.duration metric from System.Net.Http. However, it is not part of the required/recommended attributes from the spec.

v1.21
https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md#metric-httpclientduration

Main
https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md#metric-httpclientrequestduration

Reproduction Steps

Test on .NET8.0 RC1 using OTel SDK

Configure SDK

Sdk.CreateMeterProviderBuilder()
.AddMeter("System.Net.Http")
.Build()

Expected behavior

url.scheme is not included by default.

Actual behavior

url.scheme tag is added by default on http.client.request.duration metric

Sample output from sdk

Export http.client.request.duration, The duration of outbound HTTP requests., Unit: s, Meter: System.Net.Http
(2023-09-14T18:44:41.6943865Z, 2023-09-14T18:44:48.7866188Z] http.request.method: GET http.response.status_code: 302 network.protocol.version: 1.1 server.address: www.bing.com url.scheme: http Histogram

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: vishweshbankwar
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@antonfirsov
Copy link
Member

antonfirsov commented Sep 14, 2023

AFAIK the spec does not prohibit emitting additional tags. There is a proposal documentimg all .NET metrics at open-telemetry/semantic-conventions#283 where url.scheme is included as an attribute of http.client.request.duration.

Do you experience any practical disadvantage from including url.scheme @vishweshbankwar?

@antonfirsov antonfirsov reopened this Sep 14, 2023
@ghost ghost added untriaged New issue has not been triaged by the area owner and removed untriaged New issue has not been triaged by the area owner labels Sep 14, 2023
@karelz karelz added the needs-author-action An issue or pull request that requires more info or actions from the author. label Sep 19, 2023
@ghost
Copy link

ghost commented Sep 19, 2023

This issue has been marked needs-author-action and may be missing some important information.

@lmolkova
Copy link

lmolkova commented Sep 29, 2023

Created open-telemetry/semantic-conventions#356 and proposing open-telemetry/semantic-conventions#357 to fix it on the otel side

@vishweshbankwar
Copy link
Contributor Author

Closing this issue assuming the open-telemetry/semantic-conventions#356 will be merged.

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Sep 29, 2023
@karelz karelz added this to the 9.0.0 milestone Oct 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Nov 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http needs-author-action An issue or pull request that requires more info or actions from the author.
Projects
None yet
Development

No branches or pull requests

4 participants