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

[chore] Move metrics initialization in service/telemetry #11185

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

bogdandrutu
Copy link
Member

No description provided.

@bogdandrutu bogdandrutu requested review from a team and codeboten September 17, 2024 00:33
@bogdandrutu bogdandrutu force-pushed the movemetricsinit branch 3 times, most recently from 1aa31ca to 8f65a8f Compare September 17, 2024 00:35
Copy link

codecov bot commented Sep 17, 2024

Codecov Report

Attention: Patch coverage is 83.87097% with 5 lines in your changes missing coverage. Please review.

Project coverage is 91.88%. Comparing base (1339c01) to head (ae7b9cb).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
service/internal/promtest/server_util.go 71.42% 2 Missing and 2 partials ⚠️
service/service.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11185      +/-   ##
==========================================
+ Coverage   91.86%   91.88%   +0.02%     
==========================================
  Files         413      416       +3     
  Lines       19766    19864      +98     
==========================================
+ Hits        18158    18252      +94     
+ Misses       1239     1236       -3     
- Partials      369      376       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

this looks mostly ok, a big chunk of this code (all the stuff in otelinit) will be removed once #11079 lands

"go.opentelemetry.io/collector/service/telemetry/internal"
)

// disableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collector should enable
// potentially high cardinality metrics. The gate will be removed when the collector allows for view configuration.
var disableHighCardinalityMetricsfeatureGate = featuregate.GlobalRegistry().MustRegister(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be moved to globalgates instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? Where else is used? My understanding is that in globalgates we put things shared between components/modules.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, i just thought of it as a convenient location for all feature gates 🤣

@bogdandrutu bogdandrutu merged commit 6cbe5d6 into open-telemetry:main Sep 17, 2024
47 of 49 checks passed
@bogdandrutu bogdandrutu deleted the movemetricsinit branch September 17, 2024 21:00
@github-actions github-actions bot added this to the next release milestone Sep 17, 2024
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.

2 participants