feat(events): Add internal event id to event metadata #21074

ArunPiduguDD · 2024-08-14T18:11:32Z

Overview

Adds an internal event id to all events that pass through Vector. This internal id will be a UUID generated on event creation, and can be used to uniquely identify an event as it traverses through different components in a pipeline.

Testing

Added unit test

Tested using demo_logs source type

pront

Overall this looks good and simple 👍 I would like to get at least one more review/stamp on this before merging.

Another good test to have, is to generate an event at source, pass it through a transform and observe that the generated ID was preserved.

lib/vector-core/proto/event.proto

lib/vector-core/Cargo.toml

bruceg

It would be interesting to see examples of how this could be used, and I agree with the recommendation to consider V7 UUIDs, but otherwise this makes sense to me.

lib/vector-core/Cargo.toml

lib/vector-core/proto/event.proto

lib/vector-core/src/event/metadata.rs

datadog-vectordotdev · 2024-08-15T20:37:14Z

Datadog Report

Branch report: internal-event-id
Commit report: acfc653
Test service: vector

✅ 0 Failed, 7 Passed, 0 Skipped, 25.56s Total Time

jorgehermo9 · 2024-08-15T21:06:30Z

Some test fail due to the fact that EventMetadata::Default() is now not deterministic (as uuid are random). Don't know what to do to fix that without involving much changes 🤔

Maybe a manual impl of PartialEq for that struct, so that the uuid field is ignored in the comparison...

ArunPiduguDD · 2024-08-19T14:47:40Z

Some test fail due to the fact that EventMetadata::Default() is now not deterministic (as uuid are random). Don't know what to do to fix that without involving much changes 🤔

Maybe a manual impl of PartialEq for that struct, so that the uuid field is ignored in the comparison...

@jorgehermo9 ended up using the derivative crate to ignore PartialEq (saw this crate is used in other parts of the vector codebase, but lmk if you think there might be any issues introduced by this)

jorgehermo9 · 2024-08-19T14:49:20Z

yep, thats sounds like what I would do, lgtm

jorgehermo9 · 2024-08-27T21:52:52Z

I see that a lot of files were included in a30e417. Maybe should this PR be reviewed and approved again before including it in the merge queue? @bruceg

ArunPiduguDD · 2024-08-27T21:59:10Z

I see that a lot of files were included in a30e417. Maybe should this PR be reviewed and approved again before including it in the merge queue? @bruceg

Had a discussion with @bruceg and @jszwedko offline about regenerating these test fixtures, can wait for another stamp of approval before merging

jszwedko · 2024-08-27T22:17:50Z

Small nit: we generally like to discourage force-pushes to Vector PRs once reviews have started as it makes the PR history more difficult to understand. Instead of rebasing you can merge in master as needed. The PR is ultimately squashed into a single commit when merging so you don't have to worry about maintaining a clean history in the PR itself.

I'm realizing this isn't mentioned in https://github.com/vectordotdev/vector/blob/master/CONTRIBUTING.md. I'll add a note there.

bruceg

The existing code LGTM. This will need to have the new pre-v41 added to the test suite in lib/codecs/tests/native.rs however.

lib/vector-core/src/event/proto.rs

lib/vector-core/src/event/metadata.rs

bruceg · 2024-08-28T17:23:02Z

lib/codecs/tests/native.rs

+#[test]
+fn pre_v41_fixtures_match() {
+    fixtures_match("pre-v41");
+}


I think we also need tests of roundtrip_fixtures and decoding_matches.

It looks like the decoding_matches tests are broken due to #18570. Will add the ignore flag for the pre-v41 test as well unless there is an existing fix

ArunPiduguDD

/ci-run-regression

ArunPiduguDD

/ci-run-regression

jszwedko

/ci-run-regression

…1074) * add internal event id to event metadata * fix formatting * formatting * rename source_event_id + use uuid v7 + add uuid as workspace dependency * cargo fmt * use derivative crate to ignore source_event_id field for PartialEq * rebase + merge * fix Cargo.toml file * add protobuf parsing for event id field * generate new test fixtures * Update Cargo.lock * add pre-v41 to native.rs test-suit + fix import re-ordering * add pre-v41 tests for roundtrip_fixtures & decoding_matches

ArunPiduguDD added the no-changelog Changes in this PR do not need user-facing explanations in the release changelog label Aug 14, 2024

ArunPiduguDD requested review from jszwedko, bruceg and pront August 14, 2024 18:11

ArunPiduguDD requested a review from a team as a code owner August 14, 2024 18:11

github-actions bot added domain: transforms Anything related to Vector's transform components domain: core Anything related to core crates i.e. vector-core, core-common, etc and removed domain: transforms Anything related to Vector's transform components labels Aug 14, 2024

pront approved these changes Aug 14, 2024

View reviewed changes

lib/vector-core/proto/event.proto Outdated Show resolved Hide resolved

jorgehermo9 reviewed Aug 14, 2024

View reviewed changes

lib/vector-core/Cargo.toml Outdated Show resolved Hide resolved

bruceg reviewed Aug 14, 2024

View reviewed changes

lib/vector-core/Cargo.toml Outdated Show resolved Hide resolved

lib/vector-core/proto/event.proto Outdated Show resolved Hide resolved

lib/vector-core/src/event/metadata.rs Outdated Show resolved Hide resolved

ArunPiduguDD enabled auto-merge August 19, 2024 18:40

bruceg approved these changes Aug 19, 2024

View reviewed changes

ArunPiduguDD removed the request for review from jszwedko August 19, 2024 21:41

ArunPiduguDD added 7 commits August 19, 2024 17:42

add internal event id to event metadata

bde2254

fix formatting

94c3cf4

formatting

bf3cc05

rename source_event_id + use uuid v7 + add uuid as workspace dependency

07230d5

cargo fmt

1bad981

use derivative crate to ignore source_event_id field for PartialEq

d36c94f

rebase + merge

0629626

ArunPiduguDD requested review from a team as code owners August 19, 2024 22:53

github-actions bot added domain: transforms Anything related to Vector's transform components domain: sinks Anything related to the Vector's sinks domain: ci Anything related to Vector's CI environment labels Aug 19, 2024

github-actions bot removed the domain: core Anything related to core crates i.e. vector-core, core-common, etc label Aug 27, 2024

Update Cargo.lock

663d575

ArunPiduguDD added this pull request to the merge queue Aug 27, 2024

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Aug 27, 2024

ArunPiduguDD requested a review from bruceg August 27, 2024 21:59

bruceg requested changes Aug 28, 2024

View reviewed changes

lib/vector-core/src/event/proto.rs Outdated Show resolved Hide resolved

lib/vector-core/src/event/metadata.rs Outdated Show resolved Hide resolved

add pre-v41 to native.rs test-suit + fix import re-ordering

87452ff

ArunPiduguDD requested a review from bruceg August 28, 2024 17:01

bruceg reviewed Aug 28, 2024

View reviewed changes

add pre-v41 tests for roundtrip_fixtures & decoding_matches

69d0344

ArunPiduguDD requested a review from bruceg August 28, 2024 17:36

bruceg approved these changes Aug 28, 2024

View reviewed changes

Merge branch 'master' into internal-event-id

3c38965

ArunPiduguDD added this pull request to the merge queue Aug 29, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 29, 2024

ArunPiduguDD commented Aug 29, 2024

View reviewed changes

Merge branch 'master' into internal-event-id

f5d6301

ArunPiduguDD commented Aug 29, 2024

View reviewed changes

neuronull mentioned this pull request Aug 29, 2024

chore(ci): add codec test data dir to regression workflow ignore filter #21185

Merged

jszwedko reviewed Aug 29, 2024

View reviewed changes

Merge branch 'master' into internal-event-id

1ce186c

ArunPiduguDD added this pull request to the merge queue Aug 29, 2024

Merged via the queue into master with commit 06ad674 Aug 30, 2024
50 checks passed

ArunPiduguDD deleted the internal-event-id branch August 30, 2024 00:06

ArunPiduguDD mentioned this pull request Sep 10, 2024

fix(proto): Remove error log when source_event_id is not present #21257

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(events): Add internal event id to event metadata #21074

feat(events): Add internal event id to event metadata #21074

ArunPiduguDD commented Aug 14, 2024

pront left a comment

bruceg left a comment

datadog-vectordotdev bot commented Aug 15, 2024 •

edited

Loading

jorgehermo9 commented Aug 15, 2024 •

edited

Loading

ArunPiduguDD commented Aug 19, 2024 •

edited

Loading

jorgehermo9 commented Aug 19, 2024

jorgehermo9 commented Aug 27, 2024 •

edited

Loading

ArunPiduguDD commented Aug 27, 2024

jszwedko commented Aug 27, 2024

bruceg left a comment

bruceg Aug 28, 2024

ArunPiduguDD Aug 28, 2024 •

edited

Loading

ArunPiduguDD left a comment

ArunPiduguDD left a comment

jszwedko left a comment

feat(events): Add internal event id to event metadata #21074

feat(events): Add internal event id to event metadata #21074

Conversation

ArunPiduguDD commented Aug 14, 2024

Overview

Testing

pront left a comment

Choose a reason for hiding this comment

bruceg left a comment

Choose a reason for hiding this comment

datadog-vectordotdev bot commented Aug 15, 2024 • edited Loading

Datadog Report

jorgehermo9 commented Aug 15, 2024 • edited Loading

ArunPiduguDD commented Aug 19, 2024 • edited Loading

jorgehermo9 commented Aug 19, 2024

jorgehermo9 commented Aug 27, 2024 • edited Loading

ArunPiduguDD commented Aug 27, 2024

jszwedko commented Aug 27, 2024

bruceg left a comment

Choose a reason for hiding this comment

bruceg Aug 28, 2024

Choose a reason for hiding this comment

ArunPiduguDD Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

ArunPiduguDD left a comment

Choose a reason for hiding this comment

ArunPiduguDD left a comment

Choose a reason for hiding this comment

jszwedko left a comment

Choose a reason for hiding this comment

datadog-vectordotdev bot commented Aug 15, 2024 •

edited

Loading

jorgehermo9 commented Aug 15, 2024 •

edited

Loading

ArunPiduguDD commented Aug 19, 2024 •

edited

Loading

jorgehermo9 commented Aug 27, 2024 •

edited

Loading

ArunPiduguDD Aug 28, 2024 •

edited

Loading