Skip to content

Replace FIXME for flags value in ExponentialHistogram aggregation#4964

Open
chimchim89 wants to merge 1 commit intoopen-telemetry:mainfrom
chimchim89:fix-exponential-histogram-flags-doc
Open

Replace FIXME for flags value in ExponentialHistogram aggregation#4964
chimchim89 wants to merge 1 commit intoopen-telemetry:mainfrom
chimchim89:fix-exponential-histogram-flags-doc

Conversation

@chimchim89
Copy link
Contributor

Description

Replaces the two FIXME: Find the right value for flags comments in _ExponentialBucketHistogramAggregation.collect() in opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py.

Per the OpenTelemetry proto spec, flags is a bitmask where 0 means no flags are set. The only other defined value is DATA_POINT_FLAGS_NO_RECORDED_VALUE_MASK = 1, which represents a no-recorded-value condition. These code paths are only reached when a recorded value exists, so flags=0` is correct. The comments have been updated to explain this.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

This change only updates comments, no logic was changed. Verified with:

  • tox -e lint
  • tox -e precommit

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@herin049
Copy link
Contributor

I would want to wait to resolve this comment until #4916 is merged in.

@chimchim89
Copy link
Contributor Author

I would want to wait to resolve this comment until #4916 is merged in.

thanks! I'll wait for #4916 to be merged and then update this PR accordingly.

@aabmass aabmass moved this to Easy to review / merge / close in Python PR digest Mar 12, 2026
@aabmass aabmass moved this from Easy to review / merge / close to Reviewed PRs that need fixes in Python PR digest Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Reviewed PRs that need fixes

Development

Successfully merging this pull request may close these issues.

2 participants