Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pkg/ottl] Add complex indexing to paths and converters #20754

Merged

Conversation

TylerHelmuth
Copy link
Member

@TylerHelmuth TylerHelmuth commented Apr 7, 2023

Description:
Adds support for complexing indexing to OTTL:

  • Updates the grammar to support indexing the result of Converters and updates the Contexts
  • Updates contexts to support indexing maps with multiple fields and setting nested map fields.

As discussed in the issue, this PR's initial stance is to include indexing of Converters. If during review we determine that capability is not worth it we can remove it and stick only with complex indexing of Paths.

This PR does not yet add the extra checking that Contexts should be doing to ensure there are no extraneous Keys/Fields in the path. They are already doing a poor job of that so I'll submit a future PR to clean all that up.

Link to tracking Issue:
Closes #16811

Testing:
Added unit tests

Documentation:
Updated READMEs to include use of multiple indexes

@runforesight
Copy link

runforesight bot commented Apr 7, 2023

Foresight Summary

    
Major Impacts

build-and-test duration(33 minutes 4 seconds) has decreased 13 minutes 45 seconds compared to main branch avg(46 minutes 49 seconds).
View More Details

⭕  build-and-test-windows workflow has finished in 7 seconds (30 minutes 36 seconds less than main branch avg.) and finished at 12th Apr, 2023.


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

✅  telemetrygen workflow has finished in 1 minute 9 seconds and finished at 12th Apr, 2023.


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

✅  check-links workflow has finished in 1 minute 34 seconds (⚠️ 36 seconds more than main branch avg.) and finished at 12th Apr, 2023.


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

✅  changelog workflow has finished in 1 minute 41 seconds and finished at 12th Apr, 2023.


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

✅  prometheus-compliance-tests workflow has finished in 3 minutes 25 seconds (3 minutes less than main branch avg.) and finished at 12th Apr, 2023.


Job Failed Steps Tests
prometheus-compliance-tests -     🔗  N/A See Details

✅  e2e-tests workflow has finished in 10 minutes 52 seconds (3 minutes 13 seconds less than main branch avg.) and finished at 12th Apr, 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 15 minutes 44 seconds (⚠️ 5 minutes 12 seconds more than main branch avg.) and finished at 12th Apr, 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

✅  build-and-test workflow has finished in 33 minutes 4 seconds (13 minutes 45 seconds less than main branch avg.) and finished at 12th Apr, 2023.


Job Failed Steps Tests
govulncheck -     🔗  N/A See Details
setup-environment -     🔗  N/A See Details
check-codeowners -     🔗  N/A See Details
checks -     🔗  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 (connector) -     🔗  N/A See Details
lint-matrix (internal) -     🔗  N/A See Details
lint-matrix (other) -     🔗  N/A See Details
check-collector-module-version -     🔗  N/A See Details
build-examples -     🔗  N/A See Details
correctness-metrics -     🔗  N/A See Details
correctness-traces -     🔗  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, connector) -     🔗  N/A See Details
unittest-matrix (1.20, internal) -     🔗  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, extension) -     🔗  N/A See Details
unittest-matrix (1.19, connector) -     🔗  N/A See Details
unittest-matrix (1.19, internal) -     🔗  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, arm) -     🔗  N/A See Details
cross-compile (linux, arm64) -     🔗  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
rotate-milestone -     🔗  N/A See Details

🔎 See details on Foresight

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

@TylerHelmuth TylerHelmuth force-pushed the ottl-support-indexing-3 branch 3 times, most recently from fb13754 to ab2475f Compare April 7, 2023 17:13
@github-actions github-actions bot added processor/filter Filter processor processor/transform Transform processor labels Apr 7, 2023
@github-actions github-actions bot requested a review from boostchicken April 7, 2023 18:23
Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

Looks good to me overall. Nice work.

pkg/ottl/expression.go Outdated Show resolved Hide resolved
pkg/ottl/helpers.go Outdated Show resolved Hide resolved
pkg/ottl/README.md Outdated Show resolved Hide resolved
pkg/ottl/README.md Outdated Show resolved Hide resolved
pkg/ottl/contexts/internal/ottlcommon/map.go Outdated Show resolved Hide resolved
pkg/ottl/contexts/internal/ottlcommon/resource.go Outdated Show resolved Hide resolved
pkg/ottl/contexts/internal/ottlcommon/scope.go Outdated Show resolved Hide resolved
pkg/ottl/contexts/internal/ottlcommon/span.go Outdated Show resolved Hide resolved
pkg/ottl/README.md Outdated Show resolved Hide resolved
pkg/ottl/README.md Outdated Show resolved Hide resolved
pkg/ottl/README.md Outdated Show resolved Hide resolved
pkg/ottl/README.md Outdated Show resolved Hide resolved
@TylerHelmuth TylerHelmuth force-pushed the ottl-support-indexing-3 branch 3 times, most recently from 984f864 to b36c05d Compare April 11, 2023 20:08
@TylerHelmuth
Copy link
Member Author

TylerHelmuth commented Apr 11, 2023

@evan-bradley with the rebase the core logic changes now reside in expression.go (for Converters) and contexts/internal/map.go for Contexts. I was too zealous with my refactor in the other PR, not as much stuff needed to move 🤷

@TylerHelmuth TylerHelmuth force-pushed the ottl-support-indexing-3 branch 2 times, most recently from c31c961 to 9d530bf Compare April 11, 2023 20:26
pkg/ottl/contexts/internal/map.go Show resolved Hide resolved
pkg/ottl/contexts/internal/map.go Outdated Show resolved Hide resolved
pkg/ottl/internal/ottlcommon/value.go Show resolved Hide resolved
@TylerHelmuth TylerHelmuth force-pushed the ottl-support-indexing-3 branch from bc1b7c2 to 565bfc3 Compare April 12, 2023 21:12
Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think it would be a good move to have tests covering the error conditions when indexing, but I won't block on that.

pkg/ottl/contexts/internal/map.go Show resolved Hide resolved
pkg/ottl/expression.go Show resolved Hide resolved
pkg/ottl/expression_test.go Outdated Show resolved Hide resolved
pkg/ottl/expression_test.go Outdated Show resolved Hide resolved
pkg/ottl/expression_test.go Show resolved Hide resolved
pkg/ottl/contexts/internal/map_test.go Show resolved Hide resolved
pkg/ottl/contexts/internal/map.go Show resolved Hide resolved
@TylerHelmuth TylerHelmuth added the ready to merge Code review completed; ready to merge by maintainers label May 1, 2023
@TylerHelmuth TylerHelmuth force-pushed the ottl-support-indexing-3 branch from e6fd967 to 702687a Compare May 1, 2023 20:45
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Please resolve the conflict

@TylerHelmuth TylerHelmuth merged commit fe7b097 into open-telemetry:main May 3, 2023
@TylerHelmuth TylerHelmuth deleted the ottl-support-indexing-3 branch May 3, 2023 17:21
@github-actions github-actions bot added this to the next release milestone May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg/ottl processor/filter Filter processor processor/transform Transform processor ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[processor/transform] Unable to get item from slice resource attributes value
4 participants