Skip to content

Conversation

marcalff
Copy link
Member

@marcalff marcalff commented Jul 19, 2025

Fixes #3547

Changes

Please provide a brief description of the changes here.

  • Removed the unit member from class View
  • Removed the unit parameter from ViewFactory::Create() methods

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@marcalff marcalff requested a review from a team as a code owner July 19, 2025 20:52
Copy link

netlify bot commented Jul 19, 2025

Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name Link
🔨 Latest commit 4705974
🔍 Latest deploy log https://app.netlify.com/projects/opentelemetry-cpp-api-docs/deploys/6887cd56901971000707fa5a

Copy link

codecov bot commented Jul 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.02%. Comparing base (03e0452) to head (4705974).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3552      +/-   ##
==========================================
- Coverage   90.03%   90.02%   -0.01%     
==========================================
  Files         220      220              
  Lines        7069     7069              
==========================================
- Hits         6364     6363       -1     
- Misses        705      706       +1     
Files with missing lines Coverage Δ
sdk/include/opentelemetry/sdk/metrics/view/view.h 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marcalff
Copy link
Member Author

@lalitb

Please review, related to #2236

When the unit parameter was added in the view, this was a breaking change.
So, I assume it is ok to have a breaking change (in the SDK) to remove the view unit, instead of providing a deprecation path.

@lalitb lalitb requested a review from Copilot July 28, 2025 16:43
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the unit parameter from the View class and ViewFactory methods to address issue #3547. This is a breaking change that simplifies the View interface by eliminating the unit field that was apparently not needed.

  • Removed the unit member variable from the View class
  • Removed the unit parameter from all ViewFactory::Create() method overloads
  • Updated all test files and examples to use the new API without unit parameters

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
sdk/include/opentelemetry/sdk/metrics/view/view.h Removed unit member variable and constructor parameter from View class
sdk/include/opentelemetry/sdk/metrics/view/view_factory.h Removed unit parameter from ViewFactory::Create method signatures
sdk/src/metrics/view/view_factory.cc Updated ViewFactory::Create implementations to remove unit parameter handling
sdk/test/metrics/sum_aggregation_test.cc Updated test cases to use new View constructor without unit parameter
sdk/test/metrics/meter_test.cc Updated test case to use new View constructor without unit parameter
sdk/test/metrics/histogram_test.cc Updated test cases to use new View constructor without unit parameter
sdk/test/metrics/histogram_aggregation_test.cc Updated test case to use new View constructor without unit parameter
sdk/test/metrics/histogram_aggregation_benchmark.cc Updated benchmark to use new View constructor without unit parameter
examples/prometheus/main.cc Updated example to use new ViewFactory::Create without unit parameter
examples/otlp/grpc_metric_main.cc Updated example to use new ViewFactory::Create without unit parameter
examples/metrics_simple/metrics_ostream.cc Updated example to use new ViewFactory::Create without unit parameter
CHANGELOG.md Added entry documenting the breaking change

Copy link
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix.

@marcalff marcalff merged commit ac1172e into open-telemetry:main Jul 28, 2025
115 of 116 checks passed
malkia added a commit to malkia/opentelemetry-cpp that referenced this pull request Jul 28, 2025
@marcalff marcalff deleted the remove_view_unit branch October 18, 2025 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SDK] View should not have a unit

2 participants