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] Define functions using a Factory interface #18822

Conversation

evan-bradley
Copy link
Contributor

@evan-bradley evan-bradley commented Feb 20, 2023

Description:

This PR creates a Factory interface and uses it to define OTTL function factories.

Summarizing from the issue, the primary goals we have in mind for this interface are:

  • Improve the strength of OTTL API types by removing the need for an any type to pass functions to the parser.
  • Pass objects initialized by Collector components to functions to be used for things like logging, metrics, etc.
  • Provide a path for using optional interfaces to extend function factories in the future.
  • Establish standard names for functions to make it easier to keep them consistent across components.

Link to tracking Issue:

Resolves #14712

@runforesight
Copy link

runforesight bot commented Feb 20, 2023

Foresight Summary

    
Major Impacts

build-and-test duration(14 minutes 24 seconds) has decreased 32 minutes 12 seconds compared to main branch avg(46 minutes 36 seconds).
View More Details

✅  check-links workflow has finished in 49 seconds and finished at 10th Apr, 2023.


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

❌  changelog workflow has finished in 57 seconds (1 minute 18 seconds less than main branch avg.) and finished at 10th Apr, 2023. 1 job failed.


Job Failed Steps Tests
changelog Ensure ./.chloggen/*.yaml addition(s)     🔗  N/A See Details

❌  prometheus-compliance-tests workflow has finished in 1 minute 11 seconds (5 minutes 8 seconds less than main branch avg.) and finished at 10th Apr, 2023. 1 job failed.


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

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


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

❌  e2e-tests workflow has finished in 6 minutes 54 seconds (7 minutes 11 seconds less than main branch avg.) and finished at 10th Apr, 2023. 1 job failed.


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) Build Docker Image     🔗  N/A See Details
kubernetes-test (v1.23.13) -     🔗  N/A See Details

✅  load-tests workflow has finished in 6 minutes 57 seconds (3 minutes 22 seconds less than main branch avg.) and finished at 10th 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-windows workflow has finished in 6 seconds (30 minutes 38 seconds less than main branch avg.) and finished at 10th Apr, 2023.


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

⭕  build-and-test workflow has finished in 14 minutes 24 seconds (32 minutes 12 seconds less than main branch avg.) and finished at 10th Apr, 2023.


Job Failed Steps Tests
setup-environment -     🔗  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 (connector) Lint     🔗  N/A See Details
lint-matrix (internal) -     🔗  N/A See Details
lint-matrix (other) -     🔗  N/A See Details
checks -     🔗  N/A See Details
build-examples Build Examples     🔗  N/A See Details
correctness-metrics -     🔗  N/A See Details
check-codeowners -     🔗  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) Run Unit Tests     🔗  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) Run Unit Tests     🔗  N/A See Details
unittest-matrix (1.19, internal) -     🔗  N/A See Details
unittest-matrix (1.19, other) -     🔗  N/A See Details
unittest (1.20) Interpret result     🔗  N/A See Details
unittest (1.19) Interpret result     🔗  N/A See Details
lint Interpret result     🔗  N/A See Details
cross-compile -     🔗  N/A See Details
build-package -     🔗  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.

Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

I like how this looks. Left a couple questions.

pkg/ottl/ottlfuncs/func_delete_key.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_delete_key.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_delete_key.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_delete_key.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_delete_key.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_delete_key.go Outdated Show resolved Hide resolved
@evan-bradley evan-bradley force-pushed the ottl-function-factory-interface branch from eb49c1d to ae5fbcb Compare April 10, 2023 17:14
@github-actions github-actions bot requested review from jpkrohling and kovrus April 10, 2023 17:15
@runforesight
Copy link

runforesight bot commented Apr 10, 2023

Foresight Summary

    
Major Impacts

build-and-test-windows duration(6 seconds) has decreased 30 minutes 14 seconds compared to main branch avg(30 minutes 20 seconds).
View More Details

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


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

✅  check-links workflow has finished in 49 seconds and finished at 11th Apr, 2023.


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

✅  telemetrygen workflow has finished in 1 minute 19 seconds and finished at 11th Apr, 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 2 minutes 24 seconds and finished at 11th Apr, 2023.


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

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


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

✅  load-tests workflow has finished in 7 minutes 41 seconds (2 minutes 48 seconds less than main branch avg.) and finished at 11th Apr, 2023.


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

✅  e2e-tests workflow has finished in 12 minutes 40 seconds and finished at 11th 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

✅  build-and-test workflow has finished in 38 minutes 14 seconds (8 minutes 28 seconds less than main branch avg.) and finished at 11th Apr, 2023.


Job Failed Steps Tests
setup-environment -     🔗  N/A See Details
check-collector-module-version -     🔗  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
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
check-codeowners -     🔗  N/A See Details
correctness-metrics -     🔗  N/A See Details
build-examples -     🔗  N/A See Details
correctness-traces -     🔗  N/A See Details
integration-tests -     🔗  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, ppc64le) -     🔗  N/A See Details
cross-compile (linux, arm64) -     🔗  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-stable -     🔗  N/A See Details
publish-check -     🔗  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.

@evan-bradley evan-bradley force-pushed the ottl-function-factory-interface branch from 3daecea to 70c0226 Compare April 10, 2023 19:35
pkg/ottl/ottlfuncs/func_delete_key.go Outdated Show resolved Hide resolved
pkg/ottl/functions_test.go Outdated Show resolved Hide resolved
pkg/ottl/functions_test.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Outdated Show resolved Hide resolved
Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

Great work, very excited for this!

pkg/ottl/functions.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Outdated Show resolved Hide resolved
Copy link
Contributor

@MovieStoreGuy MovieStoreGuy left a comment

Choose a reason for hiding this comment

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

Spotted a couple things, need to go back a couple times to understand the logic a little more.

connector/countconnector/ottl.go Outdated Show resolved Hide resolved
internal/filter/filterottl/filter.go Outdated Show resolved Hide resolved
pkg/ottl/functions_test.go Outdated Show resolved Hide resolved
pkg/ottl/functions_test.go Show resolved Hide resolved
@evan-bradley evan-bradley force-pushed the ottl-function-factory-interface branch from bb0c93a to eb5851f Compare April 26, 2023 19:17
@evan-bradley evan-bradley force-pushed the ottl-function-factory-interface branch from 4c0d7a2 to f90604b Compare May 1, 2023 20:56
@evan-bradley evan-bradley force-pushed the ottl-function-factory-interface branch from f90604b to 9790513 Compare May 2, 2023 21:06
@evan-bradley evan-bradley force-pushed the ottl-function-factory-interface branch from eb95881 to e4b30d6 Compare May 2, 2023 22:52
@TylerHelmuth TylerHelmuth merged commit b2290e6 into open-telemetry:main May 3, 2023
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace function interfaces with a factory
4 participants