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

[exporter/elasticsearch] Implement elasticsearch.mapping.hints attribute handling for data points in OTel mapping mode #35479

Merged
merged 16 commits into from
Oct 1, 2024

Conversation

carsonip
Copy link
Contributor

@carsonip carsonip commented Sep 27, 2024

Description:

Supersedes #35348

elasticsearch.mapping.hints takes a slice of strings.
_doc_count enables emitting _doc_count for the document.
aggregate_metric_double causes histogram or exponential histogram to be emitted as aggregate_metric_double.

Link to tracking Issue:

Testing:

Documentation:

@carsonip carsonip changed the title [WIP][exporter/elasticsearch] Take mapping hint from elasticsearch.mapping.hints [exporter/elasticsearch] Implement elasticsearch.mapping.hints attribute handling for data points in OTel mapping mode Sep 30, 2024
@@ -7,15 +7,18 @@ change_type: enhancement
component: elasticsearchexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Emit _doc_count for metric documents in OTel mode when data point attribute _doc_count is true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[to reviewer] Removing changelog for previous doc count PR as this PR supersedes it. It does not make sense for the doc count PR changelog to be in the release notes of the next version if it is not relevant.

@carsonip carsonip marked this pull request as ready for review September 30, 2024 18:03
@carsonip carsonip requested a review from a team as a code owner September 30, 2024 18:03
@mwear mwear added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Sep 30, 2024
@carsonip
Copy link
Contributor Author

@mwear Thanks for the review. I've fixed the CI check. But would love eyes from @lahsivjar and @felixbarny before merging it.

Copy link
Member

@lahsivjar lahsivjar left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@andrzej-stencel andrzej-stencel 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 a bit worried that there's new functionality being added with no mention of it in the component's documentation. Isn't this functionality something users would be interested in?

@andrzej-stencel andrzej-stencel merged commit d4aa284 into open-telemetry:main Oct 1, 2024
156 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 1, 2024
jriguera pushed a commit to springernature/opentelemetry-collector-contrib that referenced this pull request Oct 4, 2024
…ute handling for data points in OTel mapping mode (open-telemetry#35479)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Supersedes open-telemetry#35348 

elasticsearch.mapping.hints takes a slice of strings.
`_doc_count` enables emitting `_doc_count` for the document.
`aggregate_metric_double` causes histogram or exponential histogram to
be emitted as aggregate_metric_double.

**Link to tracking Issue:** <Issue number if applicable>

**Testing:** <Describe what testing was performed and which tests were
added.>

**Documentation:** <Describe the documentation added.>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/elasticsearch Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants