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

Remove ecs.version field from mappings #11632

Merged
merged 1 commit into from
Sep 12, 2023
Merged

Remove ecs.version field from mappings #11632

merged 1 commit into from
Sep 12, 2023

Conversation

axw
Copy link
Member

@axw axw commented Sep 12, 2023

Motivation/summary

Remove the ecs.version field mapping from APM data streams. We don't use this field at all in APM UI, and its presence works against converging on OTel SemConv, and decoupling the index templates from apm-server.

Checklist

How to test these changes

  1. Ingest some APM data (e.g. with apmtool)
  2. Verify the ecs.version field is not present in any APM docs
  3. Verify basic APM UI functionality still works

Related issues

elastic/elasticsearch#97546

@mergify
Copy link
Contributor

mergify bot commented Sep 12, 2023

This pull request does not have a backport label. Could you fix it @axw? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-7.17 is the label to automatically backport to the 7.17 branch.
  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Sep 12, 2023
@axw axw force-pushed the rm-ecs-version branch 2 times, most recently from 8fd0c36 to ab1f697 Compare September 12, 2023 02:02
We don't use the field at all, and its presence
works against converging on OTel SemConv, and
decoupling the index templates from apm-server.
@axw axw marked this pull request as ready for review September 12, 2023 02:23
@axw axw requested a review from a team as a code owner September 12, 2023 02:23
@@ -103,7 +103,7 @@ var observerIDsPipeline = []map[string]interface{}{{
},
}}

var ecsVersionPipeline = []map[string]interface{}{{
var removeECSVersionPipeline = []map[string]interface{}{{
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, even the previous ecsVersionPipeline was removing the field ecs.version. Does this mean we were never indexing ecs.version so it was not used in the mappings anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

The ingest pipeline was there to remove the field from documents written by older versions of apm-server. The field would get added back into documents as a constant_keyword field.

I've left the removal there for now, but I don't think we'll keep that in elastic/elasticsearch#97546. We'll just dynamically map the field for older versions of apm-server - no big deal.

@axw axw merged commit eb16642 into elastic:main Sep 12, 2023
11 checks passed
@axw axw deleted the rm-ecs-version branch September 12, 2023 02:42
@axw axw added the v8.11.0 label Sep 12, 2023
@lahsivjar lahsivjar self-assigned this Oct 24, 2023
@lahsivjar
Copy link
Contributor

lahsivjar commented Oct 24, 2023

Tested successfully with BC3 that ecs.version field is not present in any APM docs.

Screenshot 2023-10-24 at 1 12 17 PM ^ All docs with `ecs.version` not present. Screenshot 2023-10-24 at 1 12 27 PM ^ All docs with `ecs.version` present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip Skip notification from the automated backport with mergify test-plan test-plan-ok v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants