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

Add feature flag to allow enabling otel for internal metrics #4912

Merged

Conversation

splunkericl
Copy link
Contributor

Description:
Previously, the variable to gate using open telemetry metrics was a constant and always set to false. This prevents any otel collector users to enable the feature.

This change will now use feature gate flag instead. The user can register the feature flag in their code base and enable the feature.

Link to tracking Issue:
#4694

Testing:
Added unit test to run otel collector with the flag enabled. Verify and assert the metrics endpoint.

Documentation:
feature gate ID variable documentation was added.

@splunkericl splunkericl requested a review from a team as a code owner February 23, 2022 18:42
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 23, 2022

CLA Signed

The committers are authorized under a signed CLA.

@project-bot project-bot bot added this to In progress in Collector Feb 23, 2022
service/telemetry.go Outdated Show resolved Hide resolved
Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

Please add a changelog entry

service/collector_test.go Outdated Show resolved Hide resolved
Collector automation moved this from In progress to Reviewer approved Feb 24, 2022
service/telemetry.go Show resolved Hide resolved
update description with suggested change

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
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.

@splunkericl please address build failure issues

@splunkericl
Copy link
Contributor Author

@codeboten I have made a couple changes to fix lint, import order and unit test. can you run the approval workflow again?

@codecov
Copy link

codecov bot commented Mar 1, 2022

Codecov Report

Merging #4912 (c9a1402) into main (52d6518) will increase coverage by 0.20%.
The diff coverage is 100.00%.

❗ Current head c9a1402 differs from pull request most recent head 406f56a. Consider uploading reports for the commit 406f56a to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4912      +/-   ##
==========================================
+ Coverage   90.82%   91.03%   +0.20%     
==========================================
  Files         181      180       -1     
  Lines       10632    10628       -4     
==========================================
+ Hits         9657     9675      +18     
+ Misses        759      736      -23     
- Partials      216      217       +1     
Impacted Files Coverage Δ
config/configtelemetry/configtelemetry.go 90.32% <ø> (ø)
service/telemetry.go 76.08% <100.00%> (+14.72%) ⬆️
component/exporter.go 100.00% <0.00%> (ø)
component/receiver.go 100.00% <0.00%> (ø)
component/component.go 100.00% <0.00%> (ø)
component/extension.go 100.00% <0.00%> (ø)
component/processor.go 100.00% <0.00%> (ø)
internal/internalinterface/internalinterface.go
model/pdata/logs.go 95.83% <0.00%> (+13.69%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52d6518...406f56a. Read the comment docs.

@splunkericl
Copy link
Contributor Author

splunkericl commented Mar 1, 2022

@dmitryax can you please take a look on the change?

@bogdandrutu bogdandrutu merged commit f2c295b into open-telemetry:main Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Collector
  
Reviewer approved
Development

Successfully merging this pull request may close these issues.

None yet

5 participants