Skip to content

Conversation

felixbarny
Copy link
Member

@felixbarny felixbarny commented Sep 1, 2025

Properly determines the target index for metric data points based on the following attributes:

  • data_stream.dataset
  • data_stream.namespace
  • elasticsearch.index

This mimics the "dynamic routing" behavior of the OTel collector's elasticsearchexporter

Part of #133057

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@elasticsearchmachine elasticsearchmachine added external-contributor Pull request authored by a developer outside the Elasticsearch team Team:StorageEngine v9.2.0 labels Sep 1, 2025
if (isPopulated()) {
return;
}
for (int i = 0, size = attributes.size(); i < size; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if these should be ordered e.g. lexicographically, so that naming is deterministic in the presence of multiple matching attributes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorting may be pretty expensive. But I see what you mean. If both elasticsearch.index and data_stream.* attributes are set, it depends on the ordering. We can probably fix that by defining a precedence and only checking for isPopulated in the beginning of the method.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this still use the first match on DATA_STREAM_DATASET or DATA_STREAM_NAMESPACE? These are user-provided, so the list order is arbitrary? Maybe a unittest will help here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I understand. The DATA_STREAM_DATASET and DATA_STREAM_NAMESPACE attributes populate different properties. The order doesn't matter as we're iterating over all attributes.

@kkrik-es
Copy link
Contributor

kkrik-es commented Sep 2, 2025

Let's update the title and description to replace routing.

Copy link
Contributor

@kkrik-es kkrik-es left a comment

Choose a reason for hiding this comment

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

Just a minor question.

@felixbarny felixbarny changed the title OTLP: data stream routing based on attributes and scope name OTLP: determine target data stream based on attributes and scope name Sep 2, 2025
@felixbarny felixbarny requested a review from a team as a code owner September 2, 2025 10:01
@felixbarny felixbarny enabled auto-merge (squash) September 2, 2025 11:19
@felixbarny felixbarny merged commit edb6690 into elastic:main Sep 2, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external-contributor Pull request authored by a developer outside the Elasticsearch team >non-issue :StorageEngine/TSDB You know, for Metrics Team:StorageEngine v9.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants