Skip to content

Add XContentParserTsidFunnel#133457

Merged
felixbarny merged 2 commits intoelastic:mainfrom
felixbarny:xcontent-tsid-funnel
Aug 25, 2025
Merged

Add XContentParserTsidFunnel#133457
felixbarny merged 2 commits intoelastic:mainfrom
felixbarny:xcontent-tsid-funnel

Conversation

@felixbarny
Copy link
Member

Part of #132566

@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label external-contributor Pull request authored by a developer outside the Elasticsearch team v9.2.0 labels Aug 25, 2025
@felixbarny felixbarny self-assigned this Aug 25, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine removed the needs:triage Requires assignment of a team area label label Aug 25, 2025
@felixbarny felixbarny requested a review from kkrik-es August 25, 2025 06:47
/**
* A funnel that extracts dimensions from an {@link XContentParser} and adds them to a {@link TsidBuilder}.
*/
class XContentParserTsidFunnel implements TsidBuilder.ThrowingTsidFunnel<XContentParser, IOException> {
Copy link
Contributor

@kkrik-es kkrik-es Aug 25, 2025

Choose a reason for hiding this comment

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

There's duplication with IndexRouting.ExtractFromSource.Builder. Consider deduplicating the logic, here or in a follow-up.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are some subtle differences that make abstracting this difficult. For example, we're using the string representation for numbers and booleans when creating a routing_path-based hash. This isn't ideal from a performance perspective but changing that for routing_path would be a breaking change. I'll merge as-is for now and we can discuss whether creating some kind of abstraction for this would still make sense given the differences.

@felixbarny felixbarny merged commit f690a21 into elastic:main Aug 25, 2025
33 checks passed
@felixbarny felixbarny deleted the xcontent-tsid-funnel branch August 25, 2025 09:21
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