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

[receiver/otlp, internal, featuregate] Add featuregate to switch to localhost defaults for server-like components #8622

Merged
merged 17 commits into from
Jan 24, 2024

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Oct 4, 2023

Description:

  • Define component.UseLocalHostAsDefaultHost in the internal/localhostgate package.
  • Define featuregate.ErrIsAlreadyRegistered error, returned by Register when a gate is already registered.
  • Adds support for the localhost gate on the OTLP receiver.

This PR does not remove the current warning in any way, we can remove this separately.

Link to tracking Issue: Updates #8510

Testing: Adds unit tests

Documentation: Document on OTLP receiver template and add related logging.

@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (5ab066e) 90.24% compared to head (cc90a90) 90.14%.

Files Patch % Lines
internal/localhostgate/featuregate.go 37.03% 15 Missing and 2 partials ⚠️
config/internal/warning.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8622      +/-   ##
==========================================
- Coverage   90.24%   90.14%   -0.11%     
==========================================
  Files         344      345       +1     
  Lines       17996    18025      +29     
==========================================
+ Hits        16240    16248       +8     
- Misses       1428     1446      +18     
- Partials      328      331       +3     

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

@mx-psi mx-psi force-pushed the mx-psi/featuregate-localhost branch 2 times, most recently from 9b0dcd9 to 7343a0b Compare October 4, 2023 15:30
@mx-psi mx-psi force-pushed the mx-psi/featuregate-localhost branch from 7343a0b to 8f14eb0 Compare October 4, 2023 15:54
component/featuregate.go Outdated Show resolved Hide resolved
@mx-psi
Copy link
Member Author

mx-psi commented Oct 5, 2023

To see the usage of this API on contrib take a look at open-telemetry/opentelemetry-collector-contrib/pull/27439

component/featuregate.go Outdated Show resolved Hide resolved
@mx-psi
Copy link
Member Author

mx-psi commented Oct 16, 2023

Adding some people that commented on the original issue as reviewers :) PTAL!

receiver/otlpreceiver/README.md Outdated Show resolved Hide resolved
receiver/otlpreceiver/otlp.go Outdated Show resolved Hide resolved
component/featuregate.go Outdated Show resolved Hide resolved
@mx-psi
Copy link
Member Author

mx-psi commented Oct 26, 2023

@jpkrohling after the last commit we now warn per component and only if the endpoint is not set. The approach is similar to what I did for the logging exporter:

// warnLogLevel is set on unmarshaling to warn users about `loglevel` usage.
warnLogLevel bool

@mx-psi mx-psi requested a review from jpkrohling October 26, 2023 11:07
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.

I just have concern with sharing the gate (code) and then removing the code.

component/featuregate.go Outdated Show resolved Hide resolved
@mx-psi
Copy link
Member Author

mx-psi commented Oct 27, 2023

@bogdandrutu see #8622 (comment) for an alternative way of doing this

receiver/otlpreceiver/otlp.go Outdated Show resolved Hide resolved
component/featuregate.go Outdated Show resolved Hide resolved
component/featuregate.go Outdated Show resolved Hide resolved
internal/localhostgate/featuregate.go Outdated Show resolved Hide resolved
internal/localhostgate/featuregate.go Outdated Show resolved Hide resolved
receiver/otlpreceiver/otlp.go Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the Stale label Nov 29, 2023

This comment was marked as outdated.

@github-actions github-actions bot added Stale and removed Stale labels Dec 13, 2023

This comment was marked as outdated.

@github-actions github-actions bot added Stale and removed Stale labels Dec 31, 2023

This comment was marked as outdated.

@github-actions github-actions bot added the Stale label Jan 20, 2024
@mx-psi
Copy link
Member Author

mx-psi commented Jan 22, 2024

@bogdandrutu (and others) will merge this by Wednesday EU morning (after this week's core release) unless somebody objects before then :)

@mx-psi mx-psi merged commit 9047c0e into open-telemetry:main Jan 24, 2024
31 of 32 checks passed
@mx-psi mx-psi deleted the mx-psi/featuregate-localhost branch January 24, 2024 12:33
@github-actions github-actions bot added this to the next release milestone Jan 24, 2024
mx-psi added a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Jan 29, 2024
**Description:** 

Follow up to open-telemetry/opentelemetry-collector/pull/8622

**Link to tracking Issue:** First step of #30702
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants