Skip to content

[chore] service/telemetry: encapsulate SDK creation#13606

Merged
mx-psi merged 1 commit into
open-telemetry:mainfrom
axw:otelconftelemetry-createtelemetry
Aug 27, 2025
Merged

[chore] service/telemetry: encapsulate SDK creation#13606
mx-psi merged 1 commit into
open-telemetry:mainfrom
axw:otelconftelemetry-createtelemetry

Conversation

@axw
Copy link
Copy Markdown
Contributor

@axw axw commented Aug 11, 2025

Description

Encapsulate opentelemetry-go SDK creation in service/telemetry/otelconftelemetry.

To enable this change, the Factory interface now provides CreateProviders method in place of the old CreateLogger, CreateMeterProvider, and CreateTracerProvider methods which have now been removed. CreateProviders returns an instance of the new telemetry.Providers interface, which provides access to telemetry providers.

In a follow-up I will move the Factory interface from service/telemetry/otelconftelemetry to service/telemetry, and inject the default level-based metric views through telemetry.Settings.

Link to tracking issue

Part of #4970

Testing

N/A, non-functional change.

Documentation

N/A

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 11, 2025

Codecov Report

❌ Patch coverage is 97.47899% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@24d9a13). Learn more about missing BASE report.
⚠️ Report is 84 commits behind head on main.

Files with missing lines Patch % Lines
service/telemetry/otelconftelemetry/telemetry.go 96.80% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #13606   +/-   ##
=======================================
  Coverage        ?   92.45%           
=======================================
  Files           ?      613           
  Lines           ?    35168           
  Branches        ?        0           
=======================================
  Hits            ?    32515           
  Misses          ?     2113           
  Partials        ?      540           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread service/service.go
Comment on lines -230 to -233
if cfg.Level == configtelemetry.LevelNone || len(cfg.Readers) == 0 {
logger.Info("Skipped telemetry setup.")
return
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This logging is now done inside otelconftelemetry.

Comment thread service/service.go
Comment on lines -235 to -239
if lmp, ok := mp.(interface {
LogAboutServers(logger *zap.Logger, cfg otelconftelemetry.MetricsConfig)
}); ok {
lmp.LogAboutServers(logger, cfg)
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Dead code, nothing implements this interface.

}
}

func TestTelemetryConfiguration(t *testing.T) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is covered by other tests that create valid/invalid loggers.

}
}

func TestSampledLogger(t *testing.T) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is covered in logger_test.go.

} else {
lp = noop.NewLoggerProvider()
}
lp := sdk.LoggerProvider()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

SDK creation is encapsulated in createProviders, which is now the only caller of newLogger. It is impossible for the SDK to be nil at this point.

@axw axw force-pushed the otelconftelemetry-createtelemetry branch from 2b6f2a3 to 000ba41 Compare August 11, 2025 09:15
Encapsulate SDK creation in otelconftelemetry.

This refactors the factory to have a single
"CreateTelemetry" method, and introduces the
unified telemetry.Telemetry interface that
provides access to telemetry providers.
@axw axw force-pushed the otelconftelemetry-createtelemetry branch from 000ba41 to 44f6ab5 Compare August 11, 2025 09:33
@axw axw marked this pull request as ready for review August 11, 2025 09:36
@axw axw requested a review from a team as a code owner August 11, 2025 09:36
@axw axw requested a review from jmacd August 11, 2025 09:36
Comment thread service/telemetry/telemetry.go
@axw axw requested a review from mx-psi August 27, 2025 00:03
Copy link
Copy Markdown
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Approving modulo the sealing interface discussion

@mx-psi
Copy link
Copy Markdown
Member

mx-psi commented Aug 27, 2025

Created #13722 to continue the dsicussion in the thread

Merged via the queue into open-telemetry:main with commit 5a5da91 Aug 27, 2025
62 of 65 checks passed
@github-actions github-actions Bot added this to the next release milestone Aug 27, 2025
andrzej-stencel added a commit to andrzej-stencel/elastic-agent that referenced this pull request Sep 12, 2025
andrzej-stencel added a commit to elastic/elastic-agent that referenced this pull request Sep 22, 2025
* feat: Update OTel Collector components to v0.135.0

* chore: update Elastic OTel components

go get github.com/elastic/opentelemetry-collector-components/connector/elasticapmconnector
go get github.com/elastic/opentelemetry-collector-components/extension/apikeyauthextension
go get github.com/elastic/opentelemetry-collector-components/extension/apmconfigextension
go get github.com/elastic/opentelemetry-collector-components/processor/elasticinframetricsprocessor
github.com/elastic/opentelemetry-collector-components/processor/elastictraceprocessor
go get github.com/elastic/opentelemetry-collector-components/receiver/elasticapmintakereceiver

* test: fix test after upstream change

Logging was changed in open-telemetry/opentelemetry-collector#13606

* test: update gotestsum to latest

Not sure why it got downgraded to v1.7.0

* use Beats from branch to verify CI

This commit must be reverted before merging this PR.
It temporarily replaces Beats dependency with elastic/beats#46464 to see if CI is happy with it,
and to fix any remaining failures without waiting for that PR to be merged.

* test: fix unit test

* revert "use Beats from branch to verify CI"

This reverts 4813ce3.

* update Beats to 16c4d9a1aa8c

This Beats commit has the update to OTel v0.135.0.

* fix collector's metric names

Without this replacement, some of the collector's internal metric names are broken.
This has been broken since:
- v9.1.3 in v9.1.x
- v8.19.3 in v8.19.x
See open-telemetry/opentelemetry-go#7039.
See https://cloud-native.slack.com/archives/C01N6P7KR6W/p1757442690145329.

* update go-mod-replace allowlist

* mage notice

---------

Co-authored-by: Christos Markou <chrismarkou92@gmail.com>
intxgo pushed a commit to intxgo/elastic-agent that referenced this pull request Sep 24, 2025
* feat: Update OTel Collector components to v0.135.0

* chore: update Elastic OTel components

go get github.com/elastic/opentelemetry-collector-components/connector/elasticapmconnector
go get github.com/elastic/opentelemetry-collector-components/extension/apikeyauthextension
go get github.com/elastic/opentelemetry-collector-components/extension/apmconfigextension
go get github.com/elastic/opentelemetry-collector-components/processor/elasticinframetricsprocessor
github.com/elastic/opentelemetry-collector-components/processor/elastictraceprocessor
go get github.com/elastic/opentelemetry-collector-components/receiver/elasticapmintakereceiver

* test: fix test after upstream change

Logging was changed in open-telemetry/opentelemetry-collector#13606

* test: update gotestsum to latest

Not sure why it got downgraded to v1.7.0

* use Beats from branch to verify CI

This commit must be reverted before merging this PR.
It temporarily replaces Beats dependency with elastic/beats#46464 to see if CI is happy with it,
and to fix any remaining failures without waiting for that PR to be merged.

* test: fix unit test

* revert "use Beats from branch to verify CI"

This reverts 4813ce3.

* update Beats to 16c4d9a1aa8c

This Beats commit has the update to OTel v0.135.0.

* fix collector's metric names

Without this replacement, some of the collector's internal metric names are broken.
This has been broken since:
- v9.1.3 in v9.1.x
- v8.19.3 in v8.19.x
See open-telemetry/opentelemetry-go#7039.
See https://cloud-native.slack.com/archives/C01N6P7KR6W/p1757442690145329.

* update go-mod-replace allowlist

* mage notice

---------

Co-authored-by: Christos Markou <chrismarkou92@gmail.com>
mergify Bot pushed a commit to elastic/elastic-agent that referenced this pull request Oct 27, 2025
* feat: Update OTel Collector components to v0.135.0

* chore: update Elastic OTel components

go get github.com/elastic/opentelemetry-collector-components/connector/elasticapmconnector
go get github.com/elastic/opentelemetry-collector-components/extension/apikeyauthextension
go get github.com/elastic/opentelemetry-collector-components/extension/apmconfigextension
go get github.com/elastic/opentelemetry-collector-components/processor/elasticinframetricsprocessor
github.com/elastic/opentelemetry-collector-components/processor/elastictraceprocessor
go get github.com/elastic/opentelemetry-collector-components/receiver/elasticapmintakereceiver

* test: fix test after upstream change

Logging was changed in open-telemetry/opentelemetry-collector#13606

* test: update gotestsum to latest

Not sure why it got downgraded to v1.7.0

* use Beats from branch to verify CI

This commit must be reverted before merging this PR.
It temporarily replaces Beats dependency with elastic/beats#46464 to see if CI is happy with it,
and to fix any remaining failures without waiting for that PR to be merged.

* test: fix unit test

* revert "use Beats from branch to verify CI"

This reverts 4813ce3.

* update Beats to 16c4d9a1aa8c

This Beats commit has the update to OTel v0.135.0.

* fix collector's metric names

Without this replacement, some of the collector's internal metric names are broken.
This has been broken since:
- v9.1.3 in v9.1.x
- v8.19.3 in v8.19.x
See open-telemetry/opentelemetry-go#7039.
See https://cloud-native.slack.com/archives/C01N6P7KR6W/p1757442690145329.

* update go-mod-replace allowlist

* mage notice

---------

Co-authored-by: Christos Markou <chrismarkou92@gmail.com>
(cherry picked from commit ecb0e06)

# Conflicts:
#	NOTICE-fips.txt
#	NOTICE.txt
#	go.mod
#	go.sum
#	internal/pkg/otel/README.md
mergify Bot pushed a commit to elastic/elastic-agent that referenced this pull request Oct 27, 2025
* feat: Update OTel Collector components to v0.135.0

* chore: update Elastic OTel components

go get github.com/elastic/opentelemetry-collector-components/connector/elasticapmconnector
go get github.com/elastic/opentelemetry-collector-components/extension/apikeyauthextension
go get github.com/elastic/opentelemetry-collector-components/extension/apmconfigextension
go get github.com/elastic/opentelemetry-collector-components/processor/elasticinframetricsprocessor
github.com/elastic/opentelemetry-collector-components/processor/elastictraceprocessor
go get github.com/elastic/opentelemetry-collector-components/receiver/elasticapmintakereceiver

* test: fix test after upstream change

Logging was changed in open-telemetry/opentelemetry-collector#13606

* test: update gotestsum to latest

Not sure why it got downgraded to v1.7.0

* use Beats from branch to verify CI

This commit must be reverted before merging this PR.
It temporarily replaces Beats dependency with elastic/beats#46464 to see if CI is happy with it,
and to fix any remaining failures without waiting for that PR to be merged.

* test: fix unit test

* revert "use Beats from branch to verify CI"

This reverts 4813ce3.

* update Beats to 16c4d9a1aa8c

This Beats commit has the update to OTel v0.135.0.

* fix collector's metric names

Without this replacement, some of the collector's internal metric names are broken.
This has been broken since:
- v9.1.3 in v9.1.x
- v8.19.3 in v8.19.x
See open-telemetry/opentelemetry-go#7039.
See https://cloud-native.slack.com/archives/C01N6P7KR6W/p1757442690145329.

* update go-mod-replace allowlist

* mage notice

---------

Co-authored-by: Christos Markou <chrismarkou92@gmail.com>
(cherry picked from commit ecb0e06)

# Conflicts:
#	NOTICE-fips.txt
#	NOTICE.txt
#	go.mod
#	go.sum
#	internal/pkg/otel/README.md
#	internal/pkg/otel/manager/manager_test.go
swiatekm pushed a commit to elastic/elastic-agent that referenced this pull request Oct 27, 2025
* feat: Update OTel Collector components to v0.135.0

* chore: update Elastic OTel components

go get github.com/elastic/opentelemetry-collector-components/connector/elasticapmconnector
go get github.com/elastic/opentelemetry-collector-components/extension/apikeyauthextension
go get github.com/elastic/opentelemetry-collector-components/extension/apmconfigextension
go get github.com/elastic/opentelemetry-collector-components/processor/elasticinframetricsprocessor
github.com/elastic/opentelemetry-collector-components/processor/elastictraceprocessor
go get github.com/elastic/opentelemetry-collector-components/receiver/elasticapmintakereceiver

* test: fix test after upstream change

Logging was changed in open-telemetry/opentelemetry-collector#13606

* test: update gotestsum to latest

Not sure why it got downgraded to v1.7.0

* use Beats from branch to verify CI

This commit must be reverted before merging this PR.
It temporarily replaces Beats dependency with elastic/beats#46464 to see if CI is happy with it,
and to fix any remaining failures without waiting for that PR to be merged.

* test: fix unit test

* revert "use Beats from branch to verify CI"

This reverts 4813ce3.

* update Beats to 16c4d9a1aa8c

This Beats commit has the update to OTel v0.135.0.

* fix collector's metric names

Without this replacement, some of the collector's internal metric names are broken.
This has been broken since:
- v9.1.3 in v9.1.x
- v8.19.3 in v8.19.x
See open-telemetry/opentelemetry-go#7039.
See https://cloud-native.slack.com/archives/C01N6P7KR6W/p1757442690145329.

* update go-mod-replace allowlist

* mage notice

---------

Co-authored-by: Christos Markou <chrismarkou92@gmail.com>
(cherry picked from commit ecb0e06)

# Conflicts:
#	NOTICE-fips.txt
#	NOTICE.txt
#	go.mod
#	go.sum
#	internal/pkg/otel/README.md
swiatekm pushed a commit to elastic/elastic-agent that referenced this pull request Oct 28, 2025
* feat: Update OTel Collector components to v0.135.0

* chore: update Elastic OTel components

go get github.com/elastic/opentelemetry-collector-components/connector/elasticapmconnector
go get github.com/elastic/opentelemetry-collector-components/extension/apikeyauthextension
go get github.com/elastic/opentelemetry-collector-components/extension/apmconfigextension
go get github.com/elastic/opentelemetry-collector-components/processor/elasticinframetricsprocessor
github.com/elastic/opentelemetry-collector-components/processor/elastictraceprocessor
go get github.com/elastic/opentelemetry-collector-components/receiver/elasticapmintakereceiver

* test: fix test after upstream change

Logging was changed in open-telemetry/opentelemetry-collector#13606

* test: update gotestsum to latest

Not sure why it got downgraded to v1.7.0

* use Beats from branch to verify CI

This commit must be reverted before merging this PR.
It temporarily replaces Beats dependency with elastic/beats#46464 to see if CI is happy with it,
and to fix any remaining failures without waiting for that PR to be merged.

* test: fix unit test

* revert "use Beats from branch to verify CI"

This reverts 4813ce3.

* update Beats to 16c4d9a1aa8c

This Beats commit has the update to OTel v0.135.0.

* fix collector's metric names

Without this replacement, some of the collector's internal metric names are broken.
This has been broken since:
- v9.1.3 in v9.1.x
- v8.19.3 in v8.19.x
See open-telemetry/opentelemetry-go#7039.
See https://cloud-native.slack.com/archives/C01N6P7KR6W/p1757442690145329.

* update go-mod-replace allowlist

* mage notice

---------

Co-authored-by: Christos Markou <chrismarkou92@gmail.com>
(cherry picked from commit ecb0e06)

# Conflicts:
#	NOTICE-fips.txt
#	NOTICE.txt
#	go.mod
#	go.sum
#	internal/pkg/otel/README.md
swiatekm added a commit to elastic/elastic-agent that referenced this pull request Oct 28, 2025
…35.0 (#10849)

* feat: Update OTel Collector components to v0.135.0 (#9858)

* feat: Update OTel Collector components to v0.135.0

* chore: update Elastic OTel components

go get github.com/elastic/opentelemetry-collector-components/connector/elasticapmconnector
go get github.com/elastic/opentelemetry-collector-components/extension/apikeyauthextension
go get github.com/elastic/opentelemetry-collector-components/extension/apmconfigextension
go get github.com/elastic/opentelemetry-collector-components/processor/elasticinframetricsprocessor
github.com/elastic/opentelemetry-collector-components/processor/elastictraceprocessor
go get github.com/elastic/opentelemetry-collector-components/receiver/elasticapmintakereceiver

* test: fix test after upstream change

Logging was changed in open-telemetry/opentelemetry-collector#13606

* test: update gotestsum to latest

Not sure why it got downgraded to v1.7.0

* use Beats from branch to verify CI

This commit must be reverted before merging this PR.
It temporarily replaces Beats dependency with elastic/beats#46464 to see if CI is happy with it,
and to fix any remaining failures without waiting for that PR to be merged.

* test: fix unit test

* revert "use Beats from branch to verify CI"

This reverts 4813ce3.

* update Beats to 16c4d9a1aa8c

This Beats commit has the update to OTel v0.135.0.

* fix collector's metric names

Without this replacement, some of the collector's internal metric names are broken.
This has been broken since:
- v9.1.3 in v9.1.x
- v8.19.3 in v8.19.x
See open-telemetry/opentelemetry-go#7039.
See https://cloud-native.slack.com/archives/C01N6P7KR6W/p1757442690145329.

* update go-mod-replace allowlist

* mage notice

---------

Co-authored-by: Christos Markou <chrismarkou92@gmail.com>
(cherry picked from commit ecb0e06)

# Conflicts:
#	NOTICE-fips.txt
#	NOTICE.txt
#	go.mod
#	go.sum
#	internal/pkg/otel/README.md

* Fix conflicts

* Fix unit tests

* Fix an api change in mockes

* Revert "Fix unit tests"

This reverts commit 81ea886.

* Bump beats dependency and fix tests

* mage notice

---------

Co-authored-by: Andrzej Stencel <andrzej.stencel@elastic.co>
Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants