Skip to content

elasticsearchexporter: refactor encoding; drop metrics support from raw/none/bodymap mapping modes #37928

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

Merged

Conversation

axw
Copy link
Contributor

@axw axw commented Feb 14, 2025

Description

Remove mappingModel, replace with encoder interface with mapping mode-specific implementations. Drop support for encoding metrics from "none", "raw", and "bodymap" mapping modes - I don't think their support was intentional.

Link to tracking issue

More refactoring related to #36092

Testing

Unit tests added. Non-functional change, except that none/raw/bodymap can no longer handle metrics.

Documentation

N/A

@axw axw force-pushed the elasticsearchexporter-refactor-encoder branch 4 times, most recently from 3a98f3d to b2b6139 Compare February 15, 2025 07:26
@axw axw changed the title elasticsearchexporter: remove mappingModel elasticsearchexporter: refactor encoding; drop metrics support from raw/none/bodymap mapping modes Feb 15, 2025
Remove mappingModel, replace with "encoder" interface
with mapping mode-specific implementations. Drop support
for encoding metrics from "none", "raw", and "bodymap"
mapping modes - I don't think their support was intentional.

(More refactoring related to
open-telemetry#36092)
@axw axw force-pushed the elasticsearchexporter-refactor-encoder branch from b2b6139 to 5a34d72 Compare February 15, 2025 10:07
@axw axw marked this pull request as ready for review February 16, 2025 01:44
@axw axw requested a review from a team as a code owner February 16, 2025 01:44
@axw axw requested a review from dehaansa February 16, 2025 01:44
Copy link
Contributor

@carsonip carsonip left a comment

Choose a reason for hiding this comment

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

thanks for cleaning up the mess! code lgtm. Looks like a good opportunity to update the README to clarify what signals are supported under which mode. Do you mind doing that?

axw added 2 commits February 20, 2025 07:14
... and add a table showing supported signals to each.
@axw axw requested a review from carsonip February 20, 2025 00:06
@axw
Copy link
Contributor Author

axw commented Feb 20, 2025

@carsonip done, PTAL

This mode may be used for compatibility with existing dashboards that work with ECS.

| Signal | `ecs` |
Copy link
Member

Choose a reason for hiding this comment

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

Maybe consolidating these separate tables into a single table could help? This could be done as a follow-up pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah maybe, I was on the fence - I started doing it in a single table and ended up splitting it when I added new sections. I don't have a strong opinion. WDYT @carsonip?

Copy link
Contributor

@carsonip carsonip Feb 21, 2025

Choose a reason for hiding this comment

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

looks fine to me now. We can refactor the docs later, especially when we start documenting OTel field mapping

@andrzej-stencel andrzej-stencel merged commit 63825ea into open-telemetry:main Feb 21, 2025
162 checks passed
@github-actions github-actions bot added this to the next release milestone Feb 21, 2025
@axw axw deleted the elasticsearchexporter-refactor-encoder branch February 21, 2025 08:32
yiquanzhou added a commit to dash0hq/opentelemetry-collector-contrib that referenced this pull request Feb 24, 2025
* main: (55 commits)
  [chore] Update core dependencies (open-telemetry#38124)
  Add kafka topics observer implementation (open-telemetry#38060)
  [exporter/splunk_hec] Mute errors from draining the response body (open-telemetry#38118)
  [chore] [exporter/splunk_hec] Remove dead code (open-telemetry#38113)
  Add support for JUnit test results (open-telemetry#37941)
  [chore] amend changelog for prometheus receiver change (open-telemetry#38109)
  [chore] Fix dead links in issue-triaging.md (open-telemetry#38105)
  [chore] fix deprecation (open-telemetry#38107)
  [exporter/coralogix] Add new batch options to Coralogix exporter (open-telemetry#38082)
  [chore][exporter/datadog] fix integration test (open-telemetry#38091)
  [chore] Update otel to unblock contrib test in core repo (open-telemetry#38100)
  [chore] Bump go-version match to 1.23 (open-telemetry#38099)
  [exporter/elasticsearch] Add _metric_names_hash to avoid metric rejections (open-telemetry#37511)
  elasticsearchexporter: refactor encoding; drop metrics support from raw/none/bodymap mapping modes (open-telemetry#37928)
  [exporter/stefexporter] Fix incorrectly implemented STEF exporter zstd compression option (open-telemetry#38089)
  [exporter/clickhouse] Add client info for identifying exporter in `system.query_log` (open-telemetry#37146)
  [chore] Prepare release 0.120.1 (open-telemetry#38055)
  [extension/httpforwarder] Shutdown should wait server exit (open-telemetry#37735)
  receiver/prometheusremotewrite: Add two fields timestamp and value. (open-telemetry#37895)
  [reciver/sqlqueryreceiver] Add support for SapASE (sybase) (open-telemetry#37773)
  ...
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.

3 participants