Skip to content

perf(codecs): reduce allocations in OTLP metric conversion#25928

Open
bruceg wants to merge 2 commits into
masterfrom
bruceg/optimize-otlp-metric-encoder
Open

perf(codecs): reduce allocations in OTLP metric conversion#25928
bruceg wants to merge 2 commits into
masterfrom
bruceg/optimize-otlp-metric-encoder

Conversation

@bruceg

@bruceg bruceg commented Jul 22, 2026

Copy link
Copy Markdown
Member

Summary

Pass metric data by ownership through the OTLP metric encode paths to avoid redundant allocations:

  • metric_event_to_export_request and split_metric_tags now consume the owned Metric/MetricTags, moving tag keys/values, metric name, buckets, and quantiles instead of cloning them (added MetricTags::into_iter_sets).
  • build_metric_tags takes its attributes by value and moves keys/values.
  • ResourceMetrics::into_event_iter (decode path) moves the metric name and drops the redundant per-metric resource/scope pre-clones.

Note that I did not include a changelog as the feature itself was noted in the changelogs and is not yet present in a release.

Vector configuration

N/A used #25923 to validate

How did you test this PR?

Running cargo bench --package codecs --features opentelemetry otlp/encode before and after the change shows an improvement in throughput of 10-52% depending on the metric data. Handling of labels/tags showed the largest improvements.

Change Type

  • Bug fix
  • New feature
  • Dependencies
  • Non-functional (chore, refactoring, docs)
  • Performance

Is this a breaking change?

  • Yes
  • No

Does this PR include user facing changes?

  • Yes. Please add a changelog fragment based on our guidelines.
  • No. A maintainer will apply the no-changelog label to this PR.

References

Notes

  • Please read our Vector contributor resources.
  • Do not hesitate to use @vectordotdev/vector to reach out to us regarding this PR.
  • Some CI checks run only after we manually approve them.
    • We recommend adding a pre-push hook, please see this template.
    • Alternatively, we recommend running the following locally before pushing to the remote branch:
      • make fmt
      • make check-clippy (if there are failures it's possible some of them can be fixed with make clippy-fix)
      • make test
  • After a review is requested, please avoid force pushes to help us review incrementally.
    • Feel free to push as many commits as you want. They will be squashed into one before merging.
    • For example, you can run git merge origin master and git push.
  • If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
    run make build-licenses to regenerate the license inventory and commit the changes (if any). More details on the dd-rust-license-tool.

Pass metric data by ownership through the OTLP metric encode and decode paths to
avoid redundant allocations:

- metric_event_to_export_request and split_metric_tags now consume the
  owned Metric/MetricTags, moving tag keys/values, metric name, buckets,
  and quantiles instead of cloning them (added MetricTags::into_iter_sets).
- build_metric_tags takes its attributes by value and moves keys/values.
- ResourceMetrics::into_event_iter (decode path) moves the metric name and
  drops the redundant per-metric resource/scope pre-clones.
@bruceg
bruceg requested a review from petere-datadog July 22, 2026 20:46
@bruceg
bruceg requested a review from a team as a code owner July 22, 2026 20:46
@bruceg bruceg added type: tech debt A code change that does not add user value. domain: performance Anything related to Vector's performance domain: codecs Anything related to Vector's codecs (encoding/decoding) no-changelog Changes in this PR do not need user-facing explanations in the release changelog domain: opentelemetry labels Jul 22, 2026
@github-actions github-actions Bot added domain: core Anything related to core crates i.e. vector-core, core-common, etc and removed domain: codecs Anything related to Vector's codecs (encoding/decoding) labels Jul 22, 2026
Comment thread lib/opentelemetry-proto/src/metrics.rs Outdated
Comment on lines +459 to +464
if key.starts_with(RESOURCE_PREFIX) {
key.drain(..RESOURCE_PREFIX.len());
push_key_value(&mut resource_attributes, key, value);
} else if key.starts_with(SCOPE_PREFIX) {
key.drain(..SCOPE_PREFIX.len());
push_key_value(&mut scope_attributes, key, value);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

strip_prefix reads a little better here. Are you able to benchmark if the &str conversion makes any difference here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It appears to cause a repeatable ~4% regression with strip_prefix over this drain setup. Ideally we could use String::replace_first or String::remove_matches but those are still unstable and don't return if they made any changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: core Anything related to core crates i.e. vector-core, core-common, etc domain: opentelemetry domain: performance Anything related to Vector's performance no-changelog Changes in this PR do not need user-facing explanations in the release changelog type: tech debt A code change that does not add user value.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants