Schema Processor Revamp [Part 1] - Operators and Migrators #35214

ankitpatel96 · 2024-09-16T17:46:16Z

This is a slice of changes from #35248

This PR is the first one to be reviewed - its broader context can be seen in the parent PR.

A Migrator does the actual migration of a piece of data - things like renaming attributes or changing the name of a signal. An Operator operates on a typed signal - things like a SpanEvent or a LogRecord. The Operator knows how to call a Migrator for its given DataType.

The next PR will detail how operators are used to build the execution pipeline for a given schemafile.

Edit: This PR is not mergeable - it conflicts with a bunch of existing code. #35267 is a stacked PR / superset of these changes and should be safe to merge

processor/schemaprocessor/internal/migrate/attributes.go

processor/schemaprocessor/internal/migrate/conditional.go

processor/schemaprocessor/internal/migrate/migrator.go

processor/schemaprocessor/internal/migrate/multi_conditional.go

processor/schemaprocessor/internal/migrate/attributes.go

MovieStoreGuy · 2024-09-23T05:39:07Z

processor/schemaprocessor/internal/operator/attributes_operators.go

+}
+
+// LogAttributeOperator is an Operator that acts on LogRecords' attributes
+type LogAttributeOperator struct {


Could I ask that new types get moved into their own file to keep files concise?

At this point there are some 11 structs in operator, so that would be a lot of files. I'd prefer to not do that - but am open to changing my mind if you feel very strongly.

processor/schemaprocessor/internal/operator/signal_name.go

processor/schemaprocessor/internal/operator/signal_interfaces.go

processor/schemaprocessor/internal/operator/attributes_operators.go

processor/schemaprocessor/internal/migrate/multi_conditional.go

processor/schemaprocessor/internal/operator/attributes_operators.go

processor/schemaprocessor/internal/operator/signal_name.go

ankitpatel96 · 2024-10-09T18:10:32Z

Sorry for the delay - I'm still actively working on this. I was out sick for a while and had some preplanned personal events. Now that I'm back - I'll be iterating on this PR

ankitpatel96

Sorry for the long delay all - but I'm back and looking forward to working through these PRs with ya'll!

I have addressed most of your comments - the major item remaining is renaming Operator. I have also gone ahead and added a Design document to this PR to help everyone understand the overall sketch of this code.

processor/schemaprocessor/internal/operator/signal_name.go

processor/schemaprocessor/internal/operator/signal_interfaces.go

ankitpatel96 · 2024-10-14T17:44:16Z

processor/schemaprocessor/internal/operator/attributes_operators.go

+	return nil
+}
+
+func (o SpanAttributeOperator) Rollback(data any) error {


processor/schemaprocessor/internal/operator/attributes_operators.go

processor/schemaprocessor/internal/migrate/conditional.go

processor/schemaprocessor/internal/operator/attributes_operators.go

ankitpatel96 · 2024-10-15T02:55:55Z

processor/schemaprocessor/internal/operator/attributes_operators.go

+}
+
+// LogAttributeOperator is an Operator that acts on LogRecords' attributes
+type LogAttributeOperator struct {


At this point there are some 11 structs in operator, so that would be a lot of files. I'd prefer to not do that - but am open to changing my mind if you feel very strongly.

processor/schemaprocessor/internal/migrate/conditional.go

processor/schemaprocessor/internal/migrate/multi_conditional.go

ankitpatel96 · 2024-10-16T22:02:18Z

I've addressed all your comments! Thanks for the thoughtful suggestions

github-actions · 2024-10-31T05:20:43Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

tigrannajaryan · 2024-10-31T14:37:41Z

processor/schemaprocessor/DESIGN.md

@@ -0,0 +1,28 @@
+# Design


Thank you for adding this document, it is very useful.

It is really awesome, and I also had no idea github support mermaid diagrams.

100% going to start doing this more often.

Yay I'm glad ya'll like it! I contributed one to the collector too at https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/internal-architecture.md. I hope to make more!

processor/schemaprocessor/internal/migrate/attributes.go

ankitpatel96 · 2024-11-07T22:47:28Z

Thank you so much for your reviews @tigrannajaryan and @MovieStoreGuy - I realize I forgot to mention that this PR is not mergeable - it conflicts with a bunch of existing code. #35267 is a stacked PR / superset of these changes and should be safe to merge (when it's done) - so would ask y'all to start looking at it when you get the chance. It's actually quite short!

Changed files from the next PR are processor/schemaprocessor/internal/changelist/changelist.go
processor/schemaprocessor/internal/translation/revision_v1.go
processor/schemaprocessor/internal/translation/revision_v1_test.go
processor/schemaprocessor/go.mod

I'm asking a maintainer if they would be willing to push a copy of this branch to the core repo so I can switch the base of the other PR to this - thus only the stacked changes would be shown.

github-actions · 2024-11-22T05:20:58Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

…35267) **Description:** <Describe what has changed.> This is a slice of changes from #35248 This PR details how operators are used to build the execution pipeline for a given schemafile. Changed files from the [previous PR](#35214) are: processor/schemaprocessor/internal/changelist/changelist.go processor/schemaprocessor/internal/translation/revision_v1.go processor/schemaprocessor/internal/translation/revision_v1_test.go processor/schemaprocessor/go.mod I'm asking a maintainer if they would be willing to push a copy of the previous PR's branch to the core repo so I can switch the base of this PR to the previous PR - thus only the stacked changes would be shown. Edit: this is apparently not easily supported - so asking reviewers to just focus on the changed files listed above. Sorry about that! **Testing:** <Describe what testing was performed and which tests were added.> Unit tests --------- Co-authored-by: Pablo Baeyens <[email protected]>

…pen-telemetry#35267) **Description:** <Describe what has changed.> This is a slice of changes from open-telemetry#35248 This PR details how operators are used to build the execution pipeline for a given schemafile. Changed files from the [previous PR](open-telemetry#35214) are: processor/schemaprocessor/internal/changelist/changelist.go processor/schemaprocessor/internal/translation/revision_v1.go processor/schemaprocessor/internal/translation/revision_v1_test.go processor/schemaprocessor/go.mod I'm asking a maintainer if they would be willing to push a copy of the previous PR's branch to the core repo so I can switch the base of this PR to the previous PR - thus only the stacked changes would be shown. Edit: this is apparently not easily supported - so asking reviewers to just focus on the changed files listed above. Sorry about that! **Testing:** <Describe what testing was performed and which tests were added.> Unit tests --------- Co-authored-by: Pablo Baeyens <[email protected]>

ankitpatel96 requested a review from MovieStoreGuy as a code owner September 16, 2024 17:46

ankitpatel96 requested a review from a team September 16, 2024 17:46

github-actions bot assigned MovieStoreGuy Sep 16, 2024

github-actions bot added the processor/schema Schema processor label Sep 16, 2024

ankitpatel96 force-pushed the ankit-schema-processor-operator branch 2 times, most recently from 453e629 to 4027afa Compare September 17, 2024 22:44

ankitpatel96 mentioned this pull request Sep 18, 2024

[chore] Schema Processor Revamp Implementation Parent PR #35248

Open

tigrannajaryan reviewed Sep 20, 2024

View reviewed changes

ankitpatel96 force-pushed the ankit-schema-processor-operator branch from 4027afa to 39b4026 Compare September 20, 2024 20:53

ankitpatel96 requested a review from a team as a code owner September 20, 2024 20:53

MovieStoreGuy reviewed Sep 23, 2024

View reviewed changes

ankitpatel96 force-pushed the ankit-schema-processor-operator branch from 39b4026 to 20ec9bc Compare September 30, 2024 20:18

ankitpatel96 force-pushed the ankit-schema-processor-operator branch from 875f10b to 4bd85ee Compare October 15, 2024 19:20

ankitpatel96 commented Oct 15, 2024

View reviewed changes

github-actions bot added the Stale label Oct 31, 2024

tigrannajaryan approved these changes Oct 31, 2024

View reviewed changes

tigrannajaryan removed the Stale label Oct 31, 2024

MovieStoreGuy approved these changes Nov 6, 2024

View reviewed changes

MovieStoreGuy added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Nov 6, 2024

ankitpatel96 force-pushed the ankit-schema-processor-operator branch from 82ac0f4 to f51e1cf Compare November 7, 2024 21:53

ankitpatel96 added 7 commits November 7, 2024 16:58

operator and migrator support

1fe52c8

update metadata.yaml

df85bba

add support for log and all type operators in operators

b51ea3b

excise Apply and Rollback in favor of Do

92699a5

simple migrator fix

49eafe1

new tests for attribute operators

935ff02

table tests and metrics

361363f

ankitpatel96 added 12 commits November 7, 2024 16:58

move to correct file

7f43ce5

format

8df413c

more operator tests

ad44a65

spanevent operator tests

8f533b0

lots of comments

0191c3e

format

8f69415

rename migrator

32b29fa

switch to generic operator interface

1c4f2b2

rename operator to transformer

4c5c381

rename file

e858e8f

add comment about ambiguous inverts

25f85da

license files :(

8b3c62b

ankitpatel96 force-pushed the ankit-schema-processor-operator branch from f51e1cf to 8b3c62b Compare November 7, 2024 22:02

change md links since there's a linter for it

21fce18

ankitpatel96 mentioned this pull request Nov 7, 2024

[chore] Schema Processor Revamp [Part 2] - ChangeList and Revision #35267

Merged

github-actions bot added the Stale label Nov 22, 2024

songy23 added never stale Issues marked with this label will be never staled and automatically removed and removed Stale labels Nov 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Schema Processor Revamp [Part 1] - Operators and Migrators #35214

Schema Processor Revamp [Part 1] - Operators and Migrators #35214

ankitpatel96 commented Sep 16, 2024 •

edited

Loading

MovieStoreGuy Sep 23, 2024

ankitpatel96 Oct 15, 2024

ankitpatel96 commented Oct 9, 2024

ankitpatel96 left a comment

ankitpatel96 Oct 14, 2024

ankitpatel96 Oct 15, 2024

ankitpatel96 commented Oct 16, 2024

github-actions bot commented Oct 31, 2024

tigrannajaryan Oct 31, 2024

MovieStoreGuy Nov 6, 2024

ankitpatel96 Nov 7, 2024 •

edited

Loading

ankitpatel96 commented Nov 7, 2024 •

edited

Loading

github-actions bot commented Nov 22, 2024

Schema Processor Revamp [Part 1] - Operators and Migrators #35214

Are you sure you want to change the base?

Schema Processor Revamp [Part 1] - Operators and Migrators #35214

Conversation

ankitpatel96 commented Sep 16, 2024 • edited Loading

MovieStoreGuy Sep 23, 2024

Choose a reason for hiding this comment

ankitpatel96 Oct 15, 2024

Choose a reason for hiding this comment

ankitpatel96 commented Oct 9, 2024

ankitpatel96 left a comment

Choose a reason for hiding this comment

ankitpatel96 Oct 14, 2024

Choose a reason for hiding this comment

ankitpatel96 Oct 15, 2024

Choose a reason for hiding this comment

ankitpatel96 commented Oct 16, 2024

github-actions bot commented Oct 31, 2024

tigrannajaryan Oct 31, 2024

Choose a reason for hiding this comment

MovieStoreGuy Nov 6, 2024

Choose a reason for hiding this comment

ankitpatel96 Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

ankitpatel96 commented Nov 7, 2024 • edited Loading

github-actions bot commented Nov 22, 2024

ankitpatel96 commented Sep 16, 2024 •

edited

Loading

ankitpatel96 Nov 7, 2024 •

edited

Loading

ankitpatel96 commented Nov 7, 2024 •

edited

Loading