[exporter/elasticsearch] Add `mapping.mode: raw` configuration option #29619

ycombinator · 2023-12-01T22:33:02Z

Description:

This PR adds a new configuration option, mapping.mode: raw, to the Elasticsearch exporter. When set, the Elasticsearch exporter will not prefix log or span attributes with Attributes. when forming the Elasticsearch document field names for these fields. Additionally, the exporter will also not prefix span events with Events.* with forming the Elasticsearch document field names for these fields.

Link to tracking Issue: Resolves #26647

Testing:

Besides adding/updating relevant unit tests in this PR, I also tested the changes in this PR against a local Elasticsearch cluster, using the following collector configurations:

Without the new mapping.mode: raw setting.

receivers:
  tcplog:
    listen_address: "0.0.0.0:54545"

processors:
  attributes:
    actions:
      - action: insert
        key: first_attribute
        value: one
      - action: insert
        key: second_attribute
        value: two

exporters:
  debug:
    verbosity: detailed
  elasticsearch:
    endpoints: [ "https://localhost:9200" ]
    user: elastic
    password: XXXXXXXX
    logs_index: otel-logs
    tls:
      insecure_skip_verify: true
    flush:
      interval: 1s

service:
  pipelines:
    logs:
      receivers: [tcplog]
      processors: [attributes]
      exporters: [debug,elasticsearch]

Resulting document in Elasticsearch:

{
  "_index": "otel-logs",
  "_id": "l1E5J4wBD9bb2EmZJuDR",
  "_score": 1,
  "_source": {
    "@timestamp": "1970-01-01T00:00:00.000000000Z",
    "Attributes": {
      "first_attribute": "one",
      "second_attribute": "two"
    },
    "Body": "bar",
    "Scope": {
      "name": "",
      "version": ""
    },
    "SeverityNumber": 0,
    "TraceFlags": 0
  }
}

With the new mapping.mode: raw setting.

receivers:
  tcplog:
    listen_address: "0.0.0.0:54545"

processors:
  attributes:
    actions:
      - action: insert
        key: first_attribute
        value: one
      - action: insert
        key: second_attribute
        value: two

exporters:
  debug:
    verbosity: detailed
  elasticsearch:
    endpoints: [ "https://localhost:9200" ]
    user: elastic
    password: XXXXXXXX
    logs_index: otel-logs
    tls:
      insecure_skip_verify: true
    flush:
      interval: 1s
    mapping:
      mode: raw

service:
  pipelines:
    logs:
      receivers: [tcplog]
      processors: [attributes]
      exporters: [debug,elasticsearch]

Resulting document in Elasticsearch:

{
  "_index": "otel-logs",
  "_id": "jlE4J4wBD9bb2EmZp-Cd",
  "_score": 1,
  "_source": {
    "@timestamp": "1970-01-01T00:00:00.000000000Z",
    "Body": "foo bar baz",
    "Scope": {
      "name": "",
      "version": ""
    },
    "SeverityNumber": 0,
    "TraceFlags": 0,
    "first_attribute": "one",
    "second_attribute": "two"
  }
}

Documentation: Documented the new configuration option in the Elasticsearch exporter's README.md.

linux-foundation-easycla · 2023-12-01T22:33:08Z

The committers listed above are authorized under a signed CLA.

✅ login: ycombinator / name: Shaunak Kashyap (c28e23a, c5d7941, a6ff601, 233e552, 30ca34a, 5d37b81, 51bef51, 161ebdb, 115f332, a870aaf, 0139e75, d7b8b4e, 13ed3db, a6f6146, 1ec5255)
✅ login: astencel-sumo / name: Andrzej Stencel (c9f6fd5, e13b484)

JaredTan95

@ycombinator Thanks for your contributions. But we have event in traces(

opentelemetry-collector-contrib/exporter/elasticsearchexporter/model.go

Line 78 in 7fc2229

document.AddEvents("Events", span.Events())

), I think we should also remove the event prefix if this option set to true.

Maybe we can add raw mode in mappging.mode as an optional, becauese the manner of removed prefix does not match to the ecs mode.

ycombinator · 2023-12-04T20:17:59Z

Thanks for the feedback, @JaredTan95. I'll rework this PR to:

replace the mapping.omit_attributes_prefix: true setting with the mappging.mode: raw setting, and
omit the Attributes. and Events. prefixes both in Elasticsearch document field names if mappging.mode: raw is set.

ycombinator · 2023-12-05T00:51:25Z

@JaredTan95 I've updated this PR based on your feedback. Please re-review when you get a chance. Thanks!

JaredTan95 · 2023-12-06T03:06:21Z

exporter/elasticsearchexporter/testdata/config.yaml

@@ -40,3 +40,5 @@ elasticsearch/log:
    max_requests: 5
  sending_queue:
    enabled: true
+  mapping:


When we added raw mode, we dropped the attributes and events prefixes by default? So this configuration can be removed?

Yes, good catch, thanks.

Fixed in 4a4a942895bbe69a594772bcf97a048280a982ee.

ycombinator · 2023-12-13T20:48:10Z

@JaredTan95 This PR is ready for another round of review. Thanks!

JaredTan95 · 2023-12-15T01:36:51Z

exporter/elasticsearchexporter/config_test.go

@@ -184,7 +184,7 @@ func TestLoadConfig(t *testing.T) {
 					MaxInterval:     1 * time.Minute,
 				},
 				Mapping: MappingsSettings{
-					Mode:  "ecs",
+					Mode:  "raw",


How about adding another raw case instead of change ecs? others LGTM

Done in bcdd9b5a36f4276c18b010516e17f3d73deae511.

JaredTan95

Thank you for your contributions~

github-actions · 2024-01-02T05:20:10Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

andrzej-stencel · 2024-01-02T12:45:39Z

exporter/elasticsearchexporter/README.md

@@ -68,6 +68,9 @@ This exporter supports sending OpenTelemetry logs to [Elasticsearch](https://www
    - `ecs`: Try to map fields defined in the
             [OpenTelemetry Semantic Conventions](https://github.com/open-telemetry/semantic-conventions)
             to [Elastic Common Schema (ECS)](https://www.elastic.co/guide/en/ecs/current/index.html).
+    - `raw`: Omit the `Attributes.` string prefixed to field names for log and 


Is it possible for a user to want both "ecs" and "raw" functionality? With this configuration, this is not possible.

Looking at the code, I'm not sure the "ecs" mapping mode actually does anything? Is there a functional difference between "none" and "ecs" mapping mode in the current state of the exporter?

Looking at the code, I'm not sure the "ecs" mapping mode actually does anything? Is there a functional difference between "none" and "ecs" mapping mode in the current state of the exporter?

Yeah, I don't see mapping mode being used at all (before this PR introduced raw). So I don't think there's any functional difference between none and ecs mapping modes today.

I looked at the PR that introduced the ecs mapping mode — #2324 — but I don't see any explanation for why it is functionally the same as raw. My guess is that ecs was introduced as part of some future-proofing work that was never done?

Given that ecs mapping mode doesn't really do anything today, I can't say that users may want both ecs and raw functionality at the same time. Perhaps we should deprecate ecs (in a separate PR) and later remove it, and then go ahead with this PR introducing raw mapping mode? If so, are there any guidelines around such deprecation and removal (I'm new to the OTel community)?

This makes sense. If the ecs mode does not do anything, then there's no point in enabling both ecs and raw at the same time.

Perhaps we should deprecate ecs (in a separate PR) and later remove it

Agree 💯

and then go ahead with this PR introducing raw mapping mode

I think both changes can be done independently, can't they? We can move on with introducing raw mode in this PR and separately create another PR to deprecate ecs. To put it yet another way, ecs removal does not block introduction of raw in my view.

As to the deprecation and removal process, the ecs mode is currently the default, so I think the following steps make sense:

Deprecate the ecs mode and at the same time change the default from ecs to none - this is a no-op change assuming both modes are functionally the same.

Remove the ecs mode after a long time - perhaps 3 months, perhaps 6 months - I suppose there's no rush here, because having this option doesn't really hurt us, right?

I don't think introducing a feature gate for this removal makes sense, given that the fixing the configuration - removing mode: ecs or changing it to mode: none - is simpler than using a feature gate.

Thanks @astencel-sumo, I've created #30441 to track the deprecation of mode: ecs and changing the default to mode: none.

I agree we can just go ahead with this PR here introducing mode: raw now itself, independently of what becomes of mode: ecs.

andrzej-stencel

🚀

ycombinator · 2024-01-22T20:36:43Z

Hi @JaredTan95, this PR is ready for merging. I don't have rights to do that; could you merge it please? Thanks!

github-actions · 2024-02-06T05:19:05Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

andrzej-stencel · 2024-02-06T22:33:14Z

It is not stale, it is ready to merge. Can we get this merged? @open-telemetry/collector-contrib-maintainer

@ycombinator

**Description:** This PR proposes adding @ycombinator as a codeowner for the `elasticsearch` exporter component, being an [employee of Elastic](https://www.linkedin.com/company/elastic-co/people/?keywords=shaunak) and also meeting the codeowner [requirements](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#requirements): 1. [Be a member of the OpenTelemetry organization.](https://github.com/open-telemetry/community/blob/main/community-membership.md#member) * https://github.com/orgs/open-telemetry/people?query=ycombinator 2. (Code Owner Discretion) It is best to have resolved an issue related to the component, contributed directly to the component, and/or review component PRs. How much interaction with the component is required before becoming a Code Owner is up to any existing Code Owners. * Resolved #26647 via #29619 * Reviewed #31553 * Contributed #31694 as follow up to #31553 * Reviewed #31848

ycombinator requested review from a team and andrzej-stencel December 1, 2023 22:33

github-actions bot assigned jpkrohling Dec 1, 2023

github-actions bot added the exporter/elasticsearch label Dec 1, 2023

github-actions bot requested a review from JaredTan95 December 1, 2023 22:33

ycombinator force-pushed the exp-es-omit-attributes-prefix branch from 3ca7980 to 62b43f8 Compare December 1, 2023 22:37

ycombinator mentioned this pull request Dec 1, 2023

Support Sending raw logs via Elasticsearch exporter when format option is set #26647

Closed

JaredTan95 reviewed Dec 2, 2023

View reviewed changes

ycombinator force-pushed the exp-es-omit-attributes-prefix branch from 62b43f8 to bd196cf Compare December 5, 2023 00:45

ycombinator changed the title ~~[exporter/elasticsearch] Add mapping.omit_attributes_prefix configuration option~~ [exporter/elasticsearch] Add mapping.mode: raw configuration option Dec 5, 2023

ycombinator requested a review from JaredTan95 December 5, 2023 00:50

JaredTan95 reviewed Dec 6, 2023

View reviewed changes

ycombinator requested a review from JaredTan95 December 7, 2023 11:05

ycombinator force-pushed the exp-es-omit-attributes-prefix branch 2 times, most recently from 4a4a942 to 7678636 Compare December 13, 2023 20:47

JaredTan95 reviewed Dec 15, 2023

View reviewed changes

JaredTan95 approved these changes Dec 19, 2023

View reviewed changes

ycombinator force-pushed the exp-es-omit-attributes-prefix branch from bcdd9b5 to e09a946 Compare December 19, 2023 01:43

github-actions bot added the Stale label Jan 2, 2024

andrzej-stencel reviewed Jan 2, 2024

View reviewed changes

github-actions bot removed the Stale label Jan 3, 2024

andrzej-stencel approved these changes Jan 11, 2024

View reviewed changes

ycombinator mentioned this pull request Jan 11, 2024

[exporter/elasticsearch] Deprecate mode: ecs configuration option and change default to mode: none #30441

Closed

ycombinator force-pushed the exp-es-omit-attributes-prefix branch 2 times, most recently from b607508 to 613ba68 Compare January 11, 2024 22:11

ycombinator and others added 8 commits January 17, 2024 14:57

Update CHANGELOG entry

115f332

Add raw mapping mode

a870aaf

Update README

0139e75

Replace mapping.omit_attributes_prefix: true with mapping.mode: raw

d7b8b4e

Adding separate test case for format: raw

13ed3db

Escape text in CHANGELOG entry

a6f6146

make gci

c9f6fd5

Fix unit tests

1ec5255

ycombinator force-pushed the exp-es-omit-attributes-prefix branch from f4ec13a to 1ec5255 Compare January 17, 2024 22:58

ycombinator requested a review from andrzej-stencel January 17, 2024 22:58

Merge branch 'main' into exp-es-omit-attributes-prefix

e13b484

andrzej-stencel approved these changes Jan 18, 2024

View reviewed changes

andrzej-stencel added the ready to merge Code review completed; ready to merge by maintainers label Jan 18, 2024

This was referenced Jan 23, 2024

Weekly Report: 2024-01-16 - 2024-01-23 #30711

Closed

Weekly Report: 2024-01-23 - 2024-01-30 #30848

Closed

github-actions bot mentioned this pull request Feb 6, 2024

Weekly Report: 2024-01-30 - 2024-02-06 #31055

Closed

github-actions bot added the Stale label Feb 6, 2024

andrzej-stencel removed the Stale label Feb 6, 2024

MovieStoreGuy merged commit aa36c85 into open-telemetry:main Feb 6, 2024
94 checks passed

github-actions bot added this to the next release milestone Feb 6, 2024

ycombinator deleted the exp-es-omit-attributes-prefix branch February 6, 2024 22:41

This was referenced Apr 2, 2024

REQUEST: New membership for ycombinator open-telemetry/community#2031

Closed

Add @ycombinator as codeowner for elasticsearchexporter #32171

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[exporter/elasticsearch] Add `mapping.mode: raw` configuration option #29619

[exporter/elasticsearch] Add `mapping.mode: raw` configuration option #29619

ycombinator commented Dec 1, 2023 •

edited

Loading

linux-foundation-easycla bot commented Dec 1, 2023 •

edited

Loading

JaredTan95 left a comment

ycombinator commented Dec 4, 2023

ycombinator commented Dec 5, 2023

JaredTan95 Dec 6, 2023

ycombinator Dec 7, 2023

ycombinator Dec 7, 2023 •

edited

Loading

ycombinator commented Dec 13, 2023

JaredTan95 Dec 15, 2023 •

edited

Loading

ycombinator Dec 18, 2023

JaredTan95 left a comment

github-actions bot commented Jan 2, 2024

andrzej-stencel Jan 2, 2024

ycombinator Jan 9, 2024

andrzej-stencel Jan 11, 2024

ycombinator Jan 11, 2024

andrzej-stencel left a comment

ycombinator commented Jan 22, 2024

github-actions bot commented Feb 6, 2024

andrzej-stencel commented Feb 6, 2024

[exporter/elasticsearch] Add mapping.mode: raw configuration option #29619

[exporter/elasticsearch] Add mapping.mode: raw configuration option #29619

Conversation

ycombinator commented Dec 1, 2023 • edited Loading

linux-foundation-easycla bot commented Dec 1, 2023 • edited Loading

JaredTan95 left a comment

Choose a reason for hiding this comment

ycombinator commented Dec 4, 2023

ycombinator commented Dec 5, 2023

JaredTan95 Dec 6, 2023

Choose a reason for hiding this comment

ycombinator Dec 7, 2023

Choose a reason for hiding this comment

ycombinator Dec 7, 2023 • edited Loading

Choose a reason for hiding this comment

ycombinator commented Dec 13, 2023

JaredTan95 Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

ycombinator Dec 18, 2023

Choose a reason for hiding this comment

JaredTan95 left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 2, 2024

andrzej-stencel Jan 2, 2024

Choose a reason for hiding this comment

ycombinator Jan 9, 2024

Choose a reason for hiding this comment

andrzej-stencel Jan 11, 2024

Choose a reason for hiding this comment

ycombinator Jan 11, 2024

Choose a reason for hiding this comment

andrzej-stencel left a comment

Choose a reason for hiding this comment

ycombinator commented Jan 22, 2024

github-actions bot commented Feb 6, 2024

andrzej-stencel commented Feb 6, 2024

[exporter/elasticsearch] Add `mapping.mode: raw` configuration option #29619

[exporter/elasticsearch] Add `mapping.mode: raw` configuration option #29619

ycombinator commented Dec 1, 2023 •

edited

Loading

linux-foundation-easycla bot commented Dec 1, 2023 •

edited

Loading

ycombinator Dec 7, 2023 •

edited

Loading

JaredTan95 Dec 15, 2023 •

edited

Loading