Skip to content

Adding EcsNamespaceProcessor #125699

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

Open
wants to merge 45 commits into
base: main
Choose a base branch
from

Conversation

eyalkoren
Copy link
Contributor

@eyalkoren eyalkoren commented Mar 26, 2025

Namespacing algorithm [EDIT 1]:

  1. start by checking whether the document is OTel or not. A document is considered OTel if:
    • resource exists as a key and the value is a map
    • resource either doesn't contain an attributes field, or contains an attributes field of type map
    • scope is either missing or a map
    • attributes is either missing or a map
    • body is either missing or a map
    • body either doesn't contain a text field, or contains a text field of type String
    • body either doesn't contain a structured field, or contains a structured field that is not of type String
  2. if it is OTel - return as is
  3. if it is not OTel:
    • create a new attributes map
    • create new resource map with one entry of which attributes is the key and a new map as its value
    • move the following top level fields (if they exist) to the new attributes map: attributes, resource, span_id, body, severity_text and trace_id
    • add the new attributes and resource maps as top level fields
    • rename special keys (e.g. span.id, log.level) to OTel-compliant names: for each, look for a value first in the nested form and if not found look for a top level dotted field. The first value that is found is used for the renamed field
    • move all keys that start with agent. or is agent or start with cloud. or is cloud or start with host. or is host to resource.attributes
    • move all remaining top level fields, other than @timestamp, trace_id, span_id, severity_text, body, attributes, resource and scope to the new attributes map
    • flatten all fields that are not arrays in attributes and resource.attributes maps

@eyalkoren eyalkoren self-assigned this Mar 27, 2025
@eyalkoren eyalkoren added >feature :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP labels Mar 27, 2025
@eyalkoren eyalkoren marked this pull request as ready for review March 27, 2025 16:06
@elasticsearchmachine
Copy link
Collaborator

Hi @eyalkoren, I've created a changelog YAML for you.

@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Mar 27, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

Thanks Eyal, I left some initial comments. I had a question about the way that we nest a document with an existing attributes field into the OTel attributes. Is this something we want to do? For example this doc:

{
  "attributes": {
    "a": "b",
    "c": [1, 2, 3]
  },
  "log.level": "1234"
}

Becomes, after processing:

{
  "resource": {
    "attributes": {}
  },
  "severity_text": "1234",
  "attributes": {
    "attributes.a": "b",
    "attributes.c": [1, 2, 3]
  }
}

Is that the desired behavior for an existing attributes field?

@eyalkoren
Copy link
Contributor Author

eyalkoren commented Mar 30, 2025

Answering the general question:

I had a question about the way that we nest a document with an existing attributes field into the OTel attributes. Is this something we want to do?

We started off by trying to be more "clever" about this and merge OTel with non-OTel. Then we had to handle lots of corner cases, like:

  • if attributes exists and is not a map - it needs to go into a new attributes map
  • if resource exists and is not a map - it needs to go into a new attributes map
  • if there was attributes map before that included a resource entry, and the top-level resource is not a map, we need to make sure that the new attributes.resource entry's value becomes an array that includes both values
  • same for resource.attribtues
  • if both span.id and span_id exist - we need to do something about it, for example: make the new span_id an array with two values

And so forth.

So last week we decided to change the way we think about it: a document is either sent by an OTel-compliant shipper, or not. If not, no reason to treat the original fields as if they have the OTel sematics. So even if it has a field that has an OTel name, we can consider it to be by chance and namespace it. If that's so- no reason to complicate things for the unlikely event where non-OTel documents contain fields with intended OTel semantics.

Assert that it's the sameInstance, and use an immutable map of
immutable maps in order to be sure that nothing changed.
@eyalkoren
Copy link
Contributor Author

Thanks for the review @joegallo 🙏
Note that there are few more additions I need to make to this PR:

  1. it was decided to add more namespaces to attribute.resource, it's just not finalized yet exactly which
  2. after adding those - I will update and add tests accordingly
  3. I need to rename the processor type to normalize_to_otel and the processor class name to NormalizeToOtelProcessor
  4. add it to documentation

I am waiting for a finalized list of keys that need to go into resource.attributes and then I will wrap it up and request a final review, which should be quite trivial.

When I run ./gradlew ":modules:ingest-ecs:check for this branch, it fails -- perhaps there's something incorrect in the build.gradle for it (or something missing, 🤷)?

gradle is definitely not a strength of mine and Elasticsearch is not the best project to get familiar with its basics 🙂
If you have any idea what doesn't seem right - let me know. Otherwise, I'll find what it is.

@flash1293
Copy link
Contributor

We discussed that we want to avoid making this processor available to users too quickly, so let's not expose it on serverless right away. According to @joegallo it's possible to filter based on the package name to achieve this.

We still plan to make this available eventually, but since discussions around details of the central /logs endpoint are still ongoing and could affect this, it makes sense to not move unnecessary quickly here, since the value of the processor will only be achieved with the combination of logs endpoint and streams API/UI changes anyway.

@joegallo
Copy link
Contributor

+1 to @flash1293's comment that we're pretty sure we can filter this out from serverless for the time being. We'll probably want to flag that this is a tech preview (or some such) in the ordinary documentation (and that documentation may come via a different PR, I'm not sure what the intent is necessarily).

There aren't any tests of this on 8.19, so there's nothing to test
this against in a yaml-rest-compat-test way.
@joegallo
Copy link
Contributor

gradle is definitely not a strength of mine and Elasticsearch is not the best project to get familiar with its basics 🙂

Heh, I know exactly what you mean. 😄

I poked and prodded things for a little while this afternoon, though, and I got it to stop being unhappy with us for now at least. I'm going to watch this new build to see if we get to a green check mark from CI -- there was a serverless failure that kept popping up which hopefully will go away now. 🤷

@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Apr 25, 2025
@eyalkoren
Copy link
Contributor Author

eyalkoren commented May 5, 2025

@joegallo I applied all requested changes from former reviews in c66663b

In addition, I added the new logic related to more accurate namespacing of resource.attributes in 7d536a7.
The changes include:

  • a small algorithm change - we are now first moving all fields to attributes, then flatten all of them and only then move the required ones to resource.attributes
  • a static set of all ECS fields that have OTel semconv counterparts that are defined as resource attribtues
  • a mechanism to automatically detect which ECS fields correspond semconv resource attributes (through tests)

The discovery of ECS fields that correspond semconv resource attributes works as follows:

  1. discovering all semconv resource attributes - scanning the entire semconv repository, parsing all its yaml files and extracting attributes from all groups that are defined as type: resource
  2. extracting all ECS fields that have OTel semconv counterparts, based on the otel attributes in the ECS field definition
  3. finding the intersection between these sets
  4. adding the agent.* fields

For now, step 1 was implemented through web crawling of the semconv repo through GitHub APIs. The API usage is straightforward and convenient, however it turned out to have two cons in order to be practical:

  1. the test needs to be run with a GitHub token through environment variable in order to avoid rate limits
  2. it requires quite a lot of HTTP requests and in order to run in a matter of seconds rather than minutes, I had to considerably complicate it with concurrency.

I will try a different approach - cloning the entire repository and do the crawling on the local disk and let's see if this makes things better.

Some feedback I that you can already provide is:

  • is this method of using statically compiled set of resource attributes the best way? should we consider using a file instead? the advantage of a file it that it's easier to automatically create when updates are required
  • if file - where to locate it? what format should it have?
  • do we need to maintain versions of this set?
  • how do we make sure that CI normally skips all tests in ResourceAttributesTests and periodically (e.g. nightly) runs ResourceAttributesTests#testAttributesSetUpToDate, notifying specific channels on errors? We have something that runs EcsDynamicTemplatesIT nightly and notifies us whenever out dynamic templates are not in sync with the latest ECS. I am not sure how it works in buildkite, all I found is ecs-dynamic-template-tests.yml, but I am not sure how this is used

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >feature serverless-linked Added by automation, don't add manually Team:Data Management Meta label for data/management team v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants