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: do not log error when receiver has no endpoint #1666

Merged
merged 2 commits into from
Apr 24, 2023
Merged

chore: do not log error when receiver has no endpoint #1666

merged 2 commits into from
Apr 24, 2023

Conversation

jacobmarble
Copy link
Contributor

Some receivers don't have an endpoint config value. No need to log a type error for these.

FWIW, setting endpoint: :1 quiets the log, but causes the Kafka receiver to fail to unmarshal its config.

Some receivers don't have an `endpoint` config value. No need to log a
type error for these.
@jacobmarble jacobmarble requested a review from a team April 18, 2023 23:01
@jaronoff97 jaronoff97 added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 19, 2023
Copy link
Contributor

@matej-g matej-g 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 good to me, but I wonder if we should then not really also change the log level to error instead of info, if the endpoint is neither non existent (nil) nor string.

@jacobmarble
Copy link
Contributor Author

This looks good to me, but I wonder if we should then not really also change the log level to error instead of info, if the endpoint is neither non existent (nil) nor string.

Great idea. See follow-on commit.

switch endpoint := endpoint.(type) {
switch e := endpoint.(type) {
case nil:
break
Copy link
Member

Choose a reason for hiding this comment

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

We can omit this break, but I understand if it was added for readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. There are a few ways to make this functional change, but this seemed the most readable.

@jaronoff97 jaronoff97 merged commit 536ad15 into open-telemetry:main Apr 24, 2023
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
…#1666)

* chore: do not log error when receiver has no endpoint

Some receivers don't have an `endpoint` config value. No need to log a
type error for these.

* chore: integrate review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants