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

BREAKING: Use seconds as default duration for FaaS duration histograms #384

Merged

Conversation

pyohannes
Copy link
Contributor

@pyohannes pyohannes commented Oct 11, 2023

Fixes #382

Changes

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.

This PR brings the FaaS metrics faas.invoke_duration, faas.init_duration, and faas.cpu.usage in line with this requirement.

As I couldn't find a place where those metrics are currently implemented or used, this change is proposed without recourse to any compatibility mechanism.

Merge requirement checklist

@pyohannes pyohannes requested review from a team October 11, 2023 16:20
@pyohannes pyohannes changed the title BREAKING: Use seconds as default duration for FaaS metrics BREAKING: Use seconds as default duration for FaaS duration histograms Oct 11, 2023
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this under the assumption that these semconv aren't used heavily and this breaking change won't actually affect any (or not many) users.

@jsuereth
Copy link
Contributor

jsuereth commented Oct 19, 2023

@pyohannes Can you open a bug against the specification asking for SCHEMA files to allow us to denote BREAKING changes where a metric changes its unit?

EDIT: Also, FYI - I check your schema_url mark. When Schema URL cannot express a breaking change, at this point we should open a collection of bugs for schema-url improvements.

@pyohannes
Copy link
Contributor Author

@jsuereth

Can you open a bug against the specification asking for SCHEMA files to allow us to denote BREAKING changes where a metric changes its unit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Use seconds as default duration for FaaS metrics
8 participants