[schema processor] Part 3 - Modifiers and Revisions #12147 #17020

MovieStoreGuy · 2022-12-14T02:02:55Z

This is a new PR of #12147

No existing changes have happened.

runforesight · 2022-12-14T02:05:05Z

Foresight Summary

Major Impacts

build-and-test-windows duration(5 seconds) has decreased 41 minutes 44 seconds compared to main branch avg(41 minutes 49 seconds).

View More Details

✅ tracegen workflow has finished in 58 seconds (1 minute 30 seconds less than `main` branch avg.) and finished at 25th Jan, 2023.

Job	Failed Steps	Tests
build-dev	- 🔗	N/A	See Details
publish-latest	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details

⭕ changelog workflow has finished in 5 seconds (2 minutes 41 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

Job	Failed Steps	Tests
changelog	- 🔗	N/A	See Details

⭕ build-and-test-windows workflow has finished in 5 seconds (41 minutes 47 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

Job	Failed Steps	Tests
windows-unittest-matrix	- 🔗	N/A	See Details
windows-unittest	- 🔗	N/A	See Details

✅ check-links workflow has finished in 43 seconds (1 minute 41 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

Job	Failed Steps	Tests
changed files	- 🔗	N/A	See Details
check-links	- 🔗	N/A	See Details

✅ telemetrygen workflow has finished in 59 seconds (2 minutes 1 second less than `main` branch avg.) and finished at 24th Feb, 2023.

Job	Failed Steps	Tests
build-dev	- 🔗	N/A	See Details
publish-latest	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details

✅ prometheus-compliance-tests workflow has finished in 11 minutes 7 seconds (⚠️ 1 minute 57 seconds more than `main` branch avg.) and finished at 24th Feb, 2023.

Job	Failed Steps	Tests
prometheus-compliance-tests	- 🔗	✅ 21 ❌ 0 ⏭ 0 🔗	See Details

✅ build-and-test workflow has finished in 53 minutes 28 seconds (15 minutes 15 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

Job	Failed Steps	Tests
correctness-metrics	- 🔗	✅ 2 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, internal)	- 🔗	✅ 561 ❌ 0 ⏭ 0 🔗	See Details
correctness-traces	- 🔗	✅ 17 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, internal)	- 🔗	✅ 561 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, extension)	- 🔗	✅ 537 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details
check-codeowners	- 🔗	N/A	See Details
check-collector-module-version	- 🔗	N/A	See Details
lint-matrix (receiver-0)	- 🔗	N/A	See Details
lint-matrix (receiver-1)	- 🔗	N/A	See Details
lint-matrix (processor)	- 🔗	N/A	See Details
lint-matrix (exporter)	- 🔗	N/A	See Details
lint-matrix (extension)	- 🔗	N/A	See Details
lint-matrix (internal)	- 🔗	N/A	See Details
lint-matrix (other)	- 🔗	N/A	See Details
build-examples	- 🔗	N/A	See Details
checks	- 🔗	N/A	See Details
integration-tests	- 🔗	N/A	See Details
unittest-matrix (1.20, receiver-0)	- 🔗	N/A	See Details
unittest-matrix (1.20, receiver-1)	- 🔗	N/A	See Details
unittest-matrix (1.20, processor)	- 🔗	N/A	See Details
unittest-matrix (1.20, exporter)	- 🔗	N/A	See Details
unittest-matrix (1.20, extension)	- 🔗	N/A	See Details
unittest-matrix (1.20, other)	- 🔗	N/A	See Details
unittest-matrix (1.19, receiver-0)	- 🔗	N/A	See Details
unittest-matrix (1.19, receiver-1)	- 🔗	N/A	See Details
unittest-matrix (1.19, processor)	- 🔗	N/A	See Details
unittest-matrix (1.19, exporter)	- 🔗	N/A	See Details
unittest-matrix (1.19, other)	- 🔗	N/A	See Details
lint	- 🔗	N/A	See Details
unittest (1.20)	- 🔗	N/A	See Details
unittest (1.19)	- 🔗	N/A	See Details
cross-compile (darwin, amd64)	- 🔗	N/A	See Details
cross-compile (darwin, arm64)	- 🔗	N/A	See Details
cross-compile (linux, 386)	- 🔗	N/A	See Details
cross-compile (linux, amd64)	- 🔗	N/A	See Details
cross-compile (linux, arm64)	- 🔗	N/A	See Details
cross-compile (linux, arm)	- 🔗	N/A	See Details
cross-compile (linux, ppc64le)	- 🔗	N/A	See Details
cross-compile (windows, 386)	- 🔗	N/A	See Details
cross-compile (windows, amd64)	- 🔗	N/A	See Details
build-package (deb)	- 🔗	N/A	See Details
build-package (rpm)	- 🔗	N/A	See Details
windows-msi	- 🔗	N/A	See Details
publish-check	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details
publish-dev	- 🔗	N/A	See Details

✅ e2e-tests workflow has finished in 17 minutes 51 seconds and finished at 24th Feb, 2023.

Job	Failed Steps	Tests
kubernetes-test (v1.26.0)	- 🔗	N/A	See Details
kubernetes-test (v1.25.3)	- 🔗	N/A	See Details
kubernetes-test (v1.24.7)	- 🔗	N/A	See Details
kubernetes-test (v1.23.13)	- 🔗	N/A	See Details

✅ load-tests workflow has finished in 28 minutes 41 seconds (⚠️ 11 minutes 29 seconds more than `main` branch avg.) and finished at 24th Feb, 2023.

Job	Failed Steps	Tests
setup-environment	- 🔗	N/A	See Details
loadtest (TestIdleMode)	- 🔗	N/A	See Details
loadtest (TestBallastMemory\|TestLog10kDPS)	- 🔗	N/A	See Details
loadtest (TestMetric10kDPS\|TestMetricsFromFile)	- 🔗	N/A	See Details
loadtest (TestMetricResourceProcessor\|TestTrace10kSPS)	- 🔗	N/A	See Details
loadtest (TestTraceNoBackend10kSPS\|TestTrace1kSPSWithAttrs)	- 🔗	N/A	See Details
loadtest (TestTraceBallast1kSPSWithAttrs\|TestTraceBallast1kSPSAddAttrs)	- 🔗	N/A	See Details
loadtest (TestTraceAttributesProcessor)	- 🔗	N/A	See Details

🔎 See details on Foresight

^{*You can configure Foresight comments in your organization settings page.}

github-actions · 2022-12-28T05:17:12Z

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

tigrannajaryan · 2023-01-03T15:20:35Z

@MovieStoreGuy I reviewed #12147 earlier. Does this need another round of review?

MovieStoreGuy · 2023-01-04T00:35:28Z

@MovieStoreGuy I reviewed #12147 earlier. Does this need another round of review?

This is the same PR but nether of us could reopen the stale PR.

github-actions · 2023-01-18T05:17:37Z

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

MovieStoreGuy · 2023-01-18T13:56:50Z

I believe I had addressed all the comments left by @tigrannajaryan in the last reviewed pinned.

I know in my delay that schema parser as adopted v1.1, however, I would like to get this out first that works with v1.0 so the semantic convetions can be defined as stable and v1.1 can be added in once this component is added to the release.

bogdandrutu

I will think more to suggest some better design/names.

processor/schemaprocessor/internal/translation/modifier_noop.go

processor/schemaprocessor/internal/translation/modifier.go

processor/schemaprocessor/internal/translation/revision.go

tigrannajaryan

Thanks for working on this @MovieStoreGuy !

processor/schemaprocessor/internal/translation/revision.go

processor/schemaprocessor/internal/translation/modifier_race_test.go

processor/schemaprocessor/internal/translation/modifier.go

tigrannajaryan · 2023-04-18T15:19:16Z

@MovieStoreGuy is this still in progress? Do you need an additional review?

MovieStoreGuy · 2023-04-19T06:14:35Z

Still in progress, I just have been unfortunate with trying to find time to resolve this.

Talking with @mx-psi is keen to help see the implementation finished and take over my current work.

mx-psi · 2023-04-19T06:59:12Z

Will discuss further with @MovieStoreGuy and have a look at this next week to see how to continue.

@MovieStoreGuy if you have the time, would you mind going through the conversations and marking as resolved any that no longer need addressing?

tigrannajaryan · 2023-04-19T13:33:29Z

@MovieStoreGuy sounds good. Let me know if you need any help from me. I think it is important that we complete this. The lack of the transform processor comes up in our discussions on how to better handle semantic convention changes.

mx-psi · 2023-05-10T16:30:50Z

processor/schemaprocessor/internal/migrate/attributes.go

+		if matched {
+			k, updated[key] = key, struct{}{}
+		} else {
+			// TODO: Since the spec hasn't decided the behavior on what


Link to an issue for deciding this in the comment?

Not yet, was raised by @tigrannajaryan in a comment so I will raise it in the morning on the spec repo (or if anyone else gets to it before me).

Added here open-telemetry/opentelemetry-specification#3497

I think we need to make a decision on open-telemetry/opentelemetry-specification#3497
Depending on the outcome the performance may be significantly impacted.

I think this must be solved before marking the component as alpha and adding it to our official builds, but still I don't think it should block merging this PR in the interest of moving forward with other parts of the implementation

processor/schemaprocessor/internal/migrate/doc.go

MovieStoreGuy · 2023-05-17T08:35:12Z

Hey @tigrannajaryan , @mx-psi ,

Are we okay to merge this PR? I wouldn't mind getting this in so I can work on the final PR for this to get an in development module out.

tigrannajaryan

LGTM

tigrannajaryan · 2023-05-17T12:49:07Z

processor/schemaprocessor/internal/translation/revision_v1.go

+}
+
+func newAttributeChangeSetSliceFromChanges(attrs ast.Attributes) *migrate.AttributeChangeSetSlice {
+	values := make([]*migrate.AttributeChangeSet, 0, 10)


Nit: since it has no particular importance I think it can be safely deleted, values := make([]*migrate.AttributeChangeSet) should be good enough.

MovieStoreGuy · 2023-05-17T13:18:55Z

Sorry, just removing some of the generics usage since I can use ast.AttributeMap where map[AttributeKey]AttributeKey was being defined without issue.

mx-psi · 2023-05-17T15:08:30Z

Let's wait until EOW to give @bogdandrutu time to review and I will merge if no new concerns are raised :)

A modifier allows mutating a signal from a previous version to the current one and vice versa. [schema processor] Adding revisions A revision handles applying the modifiers to incoming signals and converting a signal to the next version.

Following patterns similar to databases where you'd apply changes and rollback. the Migrate packages follows these designs to help make it clear what it is trying to do.

mx-psi · 2023-05-23T09:15:17Z

@bogdandrutu I am going to merge this, we won't move this out of 'In Development' until we clarify #17020 (comment) but since we have an issue tracking this on the spec this is ready to merge. Please open follow up issues if you have any comments

MovieStoreGuy requested a review from a team as a code owner December 14, 2022 02:02

MovieStoreGuy requested a review from dashpole December 14, 2022 02:02

github-actions bot assigned TylerHelmuth Dec 14, 2022

github-actions bot added the processor/schema Schema processor label Dec 14, 2022

MovieStoreGuy added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Dec 14, 2022

github-actions bot added the Stale label Dec 28, 2022

tigrannajaryan removed the Stale label Jan 3, 2023

github-actions bot added the Stale label Jan 18, 2023

MovieStoreGuy force-pushed the msg/part-3-schema-revisions branch from 0a5002c to c5b993e Compare January 18, 2023 13:15

MovieStoreGuy removed the Stale label Jan 18, 2023

MovieStoreGuy force-pushed the msg/part-3-schema-revisions branch 3 times, most recently from a82ea85 to f4911e7 Compare January 25, 2023 04:12

bogdandrutu reviewed Jan 27, 2023

View reviewed changes

tigrannajaryan reviewed Feb 2, 2023

View reviewed changes

MovieStoreGuy force-pushed the msg/part-3-schema-revisions branch from f4911e7 to e24b6b6 Compare February 6, 2023 22:40

Aneurysm9 mentioned this pull request Feb 24, 2023

resource.Default()/etc is a footgun open-telemetry/opentelemetry-go#3769

Closed

MovieStoreGuy force-pushed the msg/part-3-schema-revisions branch from 03a9705 to 14309ca Compare February 24, 2023 10:19

MovieStoreGuy force-pushed the msg/part-3-schema-revisions branch from 14309ca to 2b0a6f4 Compare April 23, 2023 21:19

MovieStoreGuy force-pushed the msg/part-3-schema-revisions branch from 2b0a6f4 to c06ba43 Compare May 3, 2023 10:53

mx-psi reviewed May 10, 2023

View reviewed changes

MovieStoreGuy mentioned this pull request May 11, 2023

[Schema] Converting values that cause conflicts open-telemetry/opentelemetry-specification#3497

Open

mx-psi approved these changes May 17, 2023

View reviewed changes

mx-psi requested review from bogdandrutu and tigrannajaryan May 17, 2023 08:53

MovieStoreGuy force-pushed the msg/part-3-schema-revisions branch from 5353b84 to 279ff09 Compare May 17, 2023 12:26

tigrannajaryan approved these changes May 17, 2023

View reviewed changes

MovieStoreGuy added the ready to merge Code review completed; ready to merge by maintainers label May 17, 2023

MovieStoreGuy added 14 commits May 23, 2023 08:49

[schema processor] Modifier

58a7133

A modifier allows mutating a signal from a previous version to the current one and vice versa. [schema processor] Adding revisions A revision handles applying the modifiers to incoming signals and converting a signal to the next version.

Adding changes

49ba868

Adding migrate package.

af63a00

Following patterns similar to databases where you'd apply changes and rollback. the Migrate packages follows these designs to help make it clear what it is trying to do.

Removing modiferies to avoid confusion

6dd9692

Fixing up naming issues

e5298f3

Adding checks in for building a revision

bf084ac

Fixing up replaces

4d3ead6

Removing transform files

78d6abf

Fixing naming

084bc74

Fixing up conflict

df76e55

Fixing typo

811463d

Updating naming

b913d18

Fixing up ast usage

285982f

FIxing up generated mod files

83c76a2

MovieStoreGuy force-pushed the msg/part-3-schema-revisions branch from 0fa49af to 83c76a2 Compare May 22, 2023 23:21

Fixing up license headers

69cf2bb

mx-psi merged commit 085abb9 into open-telemetry:main May 23, 2023
85 checks passed

github-actions bot added this to the next release milestone May 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[schema processor] Part 3 - Modifiers and Revisions #12147 #17020

[schema processor] Part 3 - Modifiers and Revisions #12147 #17020

MovieStoreGuy commented Dec 14, 2022

runforesight bot commented Dec 14, 2022 •

edited

`build-and-test-windows` duration(5 seconds) has decreased 41 minutes 44 seconds compared to main branch avg(41 minutes 49 seconds).

✅ tracegen workflow has finished in 58 seconds (1 minute 30 seconds less than `main` branch avg.) and finished at 25th Jan, 2023.

⭕ changelog workflow has finished in 5 seconds (2 minutes 41 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

⭕ build-and-test-windows workflow has finished in 5 seconds (41 minutes 47 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

✅ check-links workflow has finished in 43 seconds (1 minute 41 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

✅ telemetrygen workflow has finished in 59 seconds (2 minutes 1 second less than `main` branch avg.) and finished at 24th Feb, 2023.

✅ prometheus-compliance-tests workflow has finished in 11 minutes 7 seconds (⚠️ 1 minute 57 seconds more than `main` branch avg.) and finished at 24th Feb, 2023.

✅ build-and-test workflow has finished in 53 minutes 28 seconds (15 minutes 15 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

✅ e2e-tests workflow has finished in 17 minutes 51 seconds and finished at 24th Feb, 2023.

✅ load-tests workflow has finished in 28 minutes 41 seconds (⚠️ 11 minutes 29 seconds more than `main` branch avg.) and finished at 24th Feb, 2023.

github-actions bot commented Dec 28, 2022

tigrannajaryan commented Jan 3, 2023

MovieStoreGuy commented Jan 4, 2023

github-actions bot commented Jan 18, 2023

MovieStoreGuy commented Jan 18, 2023

bogdandrutu left a comment

tigrannajaryan left a comment

tigrannajaryan commented Apr 18, 2023

MovieStoreGuy commented Apr 19, 2023

mx-psi commented Apr 19, 2023

tigrannajaryan commented Apr 19, 2023

mx-psi May 10, 2023

MovieStoreGuy May 10, 2023

MovieStoreGuy May 11, 2023

tigrannajaryan May 11, 2023

mx-psi May 12, 2023

MovieStoreGuy commented May 17, 2023

tigrannajaryan left a comment

tigrannajaryan May 17, 2023

MovieStoreGuy commented May 17, 2023

mx-psi commented May 17, 2023

mx-psi commented May 23, 2023 •

edited

[schema processor] Part 3 - Modifiers and Revisions #12147 #17020

[schema processor] Part 3 - Modifiers and Revisions #12147 #17020

Conversation

MovieStoreGuy commented Dec 14, 2022

runforesight bot commented Dec 14, 2022 • edited

Foresight Summary

build-and-test-windows duration(5 seconds) has decreased 41 minutes 44 seconds compared to main branch avg(41 minutes 49 seconds).

✅ tracegen workflow has finished in 58 seconds (1 minute 30 seconds less than main branch avg.) and finished at 25th Jan, 2023.

⭕ changelog workflow has finished in 5 seconds (2 minutes 41 seconds less than main branch avg.) and finished at 24th Feb, 2023.

⭕ build-and-test-windows workflow has finished in 5 seconds (41 minutes 47 seconds less than main branch avg.) and finished at 24th Feb, 2023.

✅ check-links workflow has finished in 43 seconds (1 minute 41 seconds less than main branch avg.) and finished at 24th Feb, 2023.

✅ telemetrygen workflow has finished in 59 seconds (2 minutes 1 second less than main branch avg.) and finished at 24th Feb, 2023.

✅ prometheus-compliance-tests workflow has finished in 11 minutes 7 seconds (⚠️ 1 minute 57 seconds more than main branch avg.) and finished at 24th Feb, 2023.

✅ build-and-test workflow has finished in 53 minutes 28 seconds (15 minutes 15 seconds less than main branch avg.) and finished at 24th Feb, 2023.

✅ e2e-tests workflow has finished in 17 minutes 51 seconds and finished at 24th Feb, 2023.

✅ load-tests workflow has finished in 28 minutes 41 seconds (⚠️ 11 minutes 29 seconds more than main branch avg.) and finished at 24th Feb, 2023.

github-actions bot commented Dec 28, 2022

tigrannajaryan commented Jan 3, 2023

MovieStoreGuy commented Jan 4, 2023

github-actions bot commented Jan 18, 2023

MovieStoreGuy commented Jan 18, 2023

bogdandrutu left a comment

Choose a reason for hiding this comment

tigrannajaryan left a comment

Choose a reason for hiding this comment

tigrannajaryan commented Apr 18, 2023

MovieStoreGuy commented Apr 19, 2023

mx-psi commented Apr 19, 2023

tigrannajaryan commented Apr 19, 2023

mx-psi May 10, 2023

Choose a reason for hiding this comment

MovieStoreGuy May 10, 2023

Choose a reason for hiding this comment

MovieStoreGuy May 11, 2023

Choose a reason for hiding this comment

tigrannajaryan May 11, 2023

Choose a reason for hiding this comment

mx-psi May 12, 2023

Choose a reason for hiding this comment

MovieStoreGuy commented May 17, 2023

tigrannajaryan left a comment

Choose a reason for hiding this comment

tigrannajaryan May 17, 2023

Choose a reason for hiding this comment

MovieStoreGuy commented May 17, 2023

mx-psi commented May 17, 2023

mx-psi commented May 23, 2023 • edited

runforesight bot commented Dec 14, 2022 •

edited

`build-and-test-windows` duration(5 seconds) has decreased 41 minutes 44 seconds compared to main branch avg(41 minutes 49 seconds).

✅ tracegen workflow has finished in 58 seconds (1 minute 30 seconds less than `main` branch avg.) and finished at 25th Jan, 2023.

⭕ changelog workflow has finished in 5 seconds (2 minutes 41 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

⭕ build-and-test-windows workflow has finished in 5 seconds (41 minutes 47 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

✅ check-links workflow has finished in 43 seconds (1 minute 41 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

✅ telemetrygen workflow has finished in 59 seconds (2 minutes 1 second less than `main` branch avg.) and finished at 24th Feb, 2023.

✅ prometheus-compliance-tests workflow has finished in 11 minutes 7 seconds (⚠️ 1 minute 57 seconds more than `main` branch avg.) and finished at 24th Feb, 2023.

✅ build-and-test workflow has finished in 53 minutes 28 seconds (15 minutes 15 seconds less than `main` branch avg.) and finished at 24th Feb, 2023.

✅ load-tests workflow has finished in 28 minutes 41 seconds (⚠️ 11 minutes 29 seconds more than `main` branch avg.) and finished at 24th Feb, 2023.

mx-psi commented May 23, 2023 •

edited