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

Define what constitutes breaking changes for Metrics #2864

Closed
jsuereth opened this issue Oct 11, 2022 · 6 comments · Fixed by #3225
Closed

Define what constitutes breaking changes for Metrics #2864

jsuereth opened this issue Oct 11, 2022 · 6 comments · Fixed by #3225
Assignees
Labels
area:semantic-conventions Related to semantic conventions spec:metrics Related to the specification/metrics directory triaged-accepted The issue is triaged and accepted by the OTel community, one can proceed with creating a PR proposal

Comments

@jsuereth
Copy link
Contributor

We should outline what changes to metric streams (names, unit, type + attributes) constitutes a breaking change.

More details to follow

@jsuereth jsuereth added area:semantic-conventions Related to semantic conventions spec:metrics Related to the specification/metrics directory labels Oct 11, 2022
@jsuereth jsuereth self-assigned this Oct 11, 2022
@jsuereth
Copy link
Contributor Author

We'd like to tackle this question by focusing on what use cases for metrics should be preserved between OTEL instrumentation releases and expected interactions with schema_url / Telemetry Schemas.

For the first cut, we should investigate breaking behavior of:

  • Alerts (and alert thresholds)
  • Dashboards (common usage, simple queries)
  • Analytics / "data lake"

@MrAlias
Copy link
Contributor

MrAlias commented Oct 11, 2022

Does this plan to change the semantic convention stability guidelines?

@jsuereth
Copy link
Contributor Author

Yes. Specifically I want us to answer whether:

  • We can/should rely on schema_url migrations for stability or ask for major version bumps when the absence of schema_url usage would lead to a breakage. (I.e. we consider schema_url an "aide" vs. a requirement for backends)
  • Whether we loosen the definition of adding an attribute for metrics. Specifically adding an attribute that has the same value for all existing timeseries is actually a non-breaking change.
  • Discussion/guidance around things like "split", changing "unit" type, etc.

@rbailey7210 rbailey7210 added the triaged-accepted The issue is triaged and accepted by the OTel community, one can proceed with creating a PR proposal label Oct 14, 2022
@jsuereth
Copy link
Contributor Author

Here's an unfinished document that walks through the problem of stability and how I'd like to think about it. I only had time to discuss Alerting instability of metrics, specifically when is it ok to add new attributes to a metric timeseries.

Welcome people's thoughts and opinions and more scenarios to tackle in that document as we go forward.

@jsuereth
Copy link
Contributor Author

Updated a section for Metrics for our next meeting, including three topic points:

  • Should we consider explicit bucket boundary changes on histograms breaking?
  • Should we consider moving between integer / floating point values breaking?
  • Should we consider moving between Attribute types where stringified values remain the same breaking?

I have my thoughts on each, but want to discuss in the WG.

@jsuereth
Copy link
Contributor Author

jsuereth commented Nov 1, 2022

From WG + Spec SiG:

Should we consider moving between integer / floating point values breaking?

We don't enforce this in the specification today, and doing so is likely breaking or lots of churn/work. We'd like to nuance what is considered breaking to something akin to:

  • For a given timeseries, the type of points (integer / floating point) should remain the same for its lifetime.
  • When merging timeseries, if different types of points are encountered (both integer and floating point) the resulting time series should be floating point.

jsuereth added a commit to jsuereth/opentelemetry-specification that referenced this issue Feb 17, 2023
…by semconv that can be included in stability.
jmacd added a commit that referenced this issue Apr 3, 2023
Fixes #2864
Fixes #2883

## Changes

- Explicitly define what is "enforced" by stability guarantees from
Semantic conventions.
- We enforce attribute key names + types, across resource, span, metric
and log
  - We enforce span names
  - We enforce metric names, units
- Expand allowed changes to semconv to include metric attributes that do
not increase timeseries count for a given metric.

## Context

Sig discussion thread/doc
[here](https://docs.google.com/document/d/1Nvcf1wio7nDUVcrXxVUN_f8MNmcs0OzVAZLvlth1lYY/edit?usp=sharing).

---------

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
Co-authored-by: Johannes Tax <johannes@johannes.tax>
Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
Co-authored-by: Patrice Chalin <chalin@users.noreply.github.com>
Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com>
Co-authored-by: Carlos Alberto Cortez <calberto.cortez@gmail.com>
Co-authored-by: Tyler Benson <tylerbenson@gmail.com>
Co-authored-by: Joshua Carpeggiani <32605850+joshcarp@users.noreply.github.com>
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: Asaf Mesika <asaf.mesika@gmail.com>
Co-authored-by: Evan Mattson <35585003+moonbox3@users.noreply.github.com>
Co-authored-by: jack-berg <34418638+jack-berg@users.noreply.github.com>
Co-authored-by: Antoine Toulme <antoine@lunar-ocean.com>
Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
Co-authored-by: Liudmila Molkova <limolkova@microsoft.com>
Co-authored-by: Reiley Yang <reyang@microsoft.com>
Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions spec:metrics Related to the specification/metrics directory triaged-accepted The issue is triaged and accepted by the OTel community, one can proceed with creating a PR proposal
Development

Successfully merging a pull request may close this issue.

4 participants