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] [exporter/elasticsearch] remove defunct config #33803

Merged

Conversation

axw
Copy link
Contributor

@axw axw commented Jun 28, 2024

Description:

Remove exporter/elasticsearch config fields & mentions in the README. Nothing has ever used this config.
They were added in #2324, but were not used.

If one were to add fields to all documents, then I think that should be done with a processor.

Link to tracking Issue:

N/A

Testing:

N/A

Documentation:

N/A

Nothing has ever used this config.
Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

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

I'm slightly concerned that some users are using these config options and will run into errors on upgrade. I realize they don't do anything, but it may be good to add a breaking changelog just to let users know what's going on here.

I'll defer to code owners though.

@axw
Copy link
Contributor Author

axw commented Jul 4, 2024

@crobert-1 thanks, good catch. I forgot that the config validation is strict, and would error on unknown keys. I'll add a breaking change notice.

@jpkrohling jpkrohling merged commit 344e4f2 into open-telemetry:main Jul 8, 2024
154 checks passed
@github-actions github-actions bot added this to the next release milestone Jul 8, 2024
@axw axw deleted the elasticsearch-rm-mapping-fields-file branch July 8, 2024 09:50
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.

6 participants