Skip to content

Conversation

@adiwab
Copy link

@adiwab adiwab commented Sep 1, 2025

#14015

Summary

  1. Event-time handling (absolute metrics in Auto and Latest/Diff):

    • Now compares event timestamps and only replaces the stored value if the new sample is newer (or equal).
    • Prevents late/out-of-order samples from rolling back bucket values (Event clock only).
  2. Processing-time path:

    • Absolute metrics in Auto and Latest/Diff are still replaced by the last arriving sample (no event-timestamp check yet).
  3. Other aggregation modes preserved:

    • Sum, Count for incremental metrics unchanged.
    • Max, Min, Mean, Stdev continue to aggregate multiple samples per bucket.
    • Diff emits the difference between the current and previous bucket values.
  4. New configuration options:

    • clock: choose bucket alignment by Processing (Vector wall clock) or Event (event timestamps).
    • allowed_lateness_ms: wait time for late or out-of-order samples in event-time mode (default: 120s).
    • emit_ts: set output timestamp to BucketStart or BucketEnd.

Important Notes

  • This is a draft implementation.
  • It requires thorough testing (unit and integration) to verify correctness across modes, especially with late or out-of-order samples.
  • There may be room for further optimization (e.g., reducing allocations, refining how late samples are handled).

Vector configuration

prom_aggregate:
type: aggregate
inputs:
- prometheus
interval_ms: 60000
mode: Latest
clock: Event
allowed_lateness_ms: 60000

How did you test this PR?

I built the changes locally (win11 - Build 26100) and verified the results by sending metrics into InfluxDB and inspecting them in Grafana dashboards.

Change Type

  • Bug fix
  • New feature
  • 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:
      • cargo fmt --all
      • cargo clippy --workspace --all-targets -- -D warnings
      • cargo nextest run --workspace (alternatively, you can run cargo test --all)
  • 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 cargo vdev build licenses to regenerate the license inventory and commit the changes (if any). More details here.

@adiwab adiwab requested review from a team as code owners September 1, 2025 17:54
@github-actions github-actions bot added domain: transforms Anything related to Vector's transform components domain: external docs Anything related to Vector's external, public documentation labels Sep 1, 2025
@pront
Copy link
Member

pront commented Sep 5, 2025

Hi @adiwab, the PR title feat! indicates this a breaking change (https://www.conventionalcommits.org/en/v1.0.0/), but I don't think this is true.

@pront pront added the meta: awaiting author Pull requests that are awaiting their author. label Sep 5, 2025
@adiwab adiwab changed the title feat!: Event-time based aggregation support feat: Event-time based aggregation support Sep 6, 2025
@kaarolch
Copy link
Contributor

kaarolch commented Oct 7, 2025

Hi @adiwab any update here?
Once again thank you for rising this topic, I hit the same aggregation issue during test of replace/aggreate host tag in metrics, when windows base on local time vector generate duplicated series due to send the same timestamp in multiple batches.

@pront pront changed the title feat: Event-time based aggregation support feat(aggregate transform): Event-time based aggregation support Oct 7, 2025
@adiwab
Copy link
Author

adiwab commented Oct 16, 2025

Hi @kaarolch, thanks again for your feedback and for raising this issue!

Just to clarify: my contribution was initially more of a draft. I’ve tested it to some extent, but not across all use cases, and it definitely needs someone to review it — especially since I’m not a Rust expert. Proper tests are also still missing and would need to be added before considering it a complete or production-ready solution.

Is there anyone who might be able to help refine and complete this? I'd really appreciate support from someone more experienced with Rust or the specific aggregation logic.

regards
Adrian

@kaarolch
Copy link
Contributor

Hi @adiwab! I've created a separate PR with a few edge case fixes. I hope you don't mind 🙂

@adiwab
Copy link
Author

adiwab commented Dec 30, 2025

Hi @kaarolch! Thanks, no worries at all 🙂
I’ve had a look already — looking forward to the release!

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

Labels

domain: external docs Anything related to Vector's external, public documentation domain: transforms Anything related to Vector's transform components meta: awaiting author Pull requests that are awaiting their author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants