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

Use seconds as default duration for DB client metrics #381

Closed
pyohannes opened this issue Oct 11, 2023 · 3 comments · Fixed by #966
Closed

Use seconds as default duration for DB client metrics #381

pyohannes opened this issue Oct 11, 2023 · 3 comments · Fixed by #966
Assignees

Comments

@pyohannes
Copy link
Contributor

This issue was created based on a discussion about #176.

General guidelines for metric instrumentation units require that for measuring durations, seconds should be used:

  • When instruments are measuring durations, seconds (i.e. s) SHOULD be used.

However, the following DB client metrics still use milliseconds for measuring durations:

For consistency, those histograms should be changed to use seconds. Those metrics are already in use, and to avoid unnecessary disruptions the change should happen during stabilization of DB semantic conventions and should be covered by the OTEL_SEMCONV_STABILITY_OPT_IN mechanism.

@trask
Copy link
Member

trask commented Apr 23, 2024

the initial database semantic convention stability isn't targeting these metrics, but it makes sense to me to make these changes as part of the stability effort so that they can be grouped with the other database semconv breaking changes

@jack-berg
Copy link
Member

the initial database semantic convention stability isn't targeting these metrics, but it makes sense to me to make these changes as part of the stability effort so that they can be grouped with the other database semconv breaking changes

I agree. Consumers of db semantic conventions will already be dealing with significant breaking changes. Withholding changes to part of the conventions because they're not targeted for stability is probably detrimental to the user experience.

@gregkalapos
Copy link
Member

As suggested by @trask #966 (review) doing it in #966

@gregkalapos gregkalapos moved this from Todo to In Progress in Database Client Semantic Conventions Apr 29, 2024
@trask trask moved this from In Progress to Todo in Database Client Semantic Conventions Apr 30, 2024
@trask trask moved this from Todo to In Progress in Database Client Semantic Conventions Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment