telemetry: add support for the OTel provider#40032
Merged
istio-testing merged 5 commits intoistio:masterfrom Sep 2, 2022
Merged
telemetry: add support for the OTel provider#40032istio-testing merged 5 commits intoistio:masterfrom
istio-testing merged 5 commits intoistio:masterfrom
Conversation
Collaborator
|
Skipping CI for Draft Pull Request. |
zirain
reviewed
Jul 21, 2022
ed81b87 to
9232008
Compare
c79fb6d to
1ca8b68
Compare
Contributor
Author
|
@smithclay @zirain @howardjohn please take another look. i think this might allow us to support the transition across upgrades, etc. |
1ca8b68 to
37e911e
Compare
Contributor
|
Not all Istio distributions come with all extensions. If you want to be extra strict, you could check node metadata for whether an extension has been compiled-in. |
908fd34 to
cf81429
Compare
3caa864 to
e1f9b6d
Compare
ericvn
reviewed
Aug 31, 2022
zirain
reviewed
Sep 1, 2022
Co-authored-by: Eric Van Norman <ericvn@us.ibm.com>
9c328cb to
f0499c3
Compare
ericvn
approved these changes
Sep 1, 2022
Contributor
ericvn
left a comment
There was a problem hiding this comment.
Tests and release note looks good
hzxuzhonghu
reviewed
Sep 2, 2022
| useOTel := util.IsIstioVersionGE116(model.ParseIstioVersion(meta.IstioVersion)) | ||
| if useOTel { | ||
| tracing, err = buildHCMTracing(pushCtx, envoyOpenTelemetry, provider.Lightstep.Service, provider.Lightstep.Port, provider.Lightstep.MaxTagLength, | ||
| func(hostname, clusterName string) (*anypb.Any, error) { |
Member
There was a problem hiding this comment.
nit: To be consistent, abstract a function as other provider
could be in a follow up
hzxuzhonghu
approved these changes
Sep 2, 2022
thiagogcm
reviewed
Sep 2, 2022
| // for lightstep for everything before 1.16, but OTel-based configuration for all other cases | ||
| useOTel := util.IsIstioVersionGE116(model.ParseIstioVersion(meta.IstioVersion)) | ||
| if useOTel { | ||
| tracing, err = buildHCMTracing(pushCtx, envoyOpenTelemetry, provider.Lightstep.Service, provider.Lightstep.Port, provider.Lightstep.MaxTagLength, |
There was a problem hiding this comment.
is the provider correct here?
or provider.Opentelemetry.Service, provider.Opentelemetry.Port..?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds support for the new OpenTelemetry tracer extension in Envoy. At the same time, it migrates Lightstep providers to the new OpenTelemetry provider (as the Lightstep tracer extension has been removed from Envoy) for future versions of Istio 1.16+.
In pursuit of this goal, the PR also updates the generated tracing config to match envoy documentation (envoy configuration provider name should match envoy extension name), and refactors an existing test suite for OpenCensus agents to also handle OTLP integrations. Additionally, the OTel collector testing component is updated to a more recent version.
To support OTel integrations, an Envoy gRPC endpoint is configured with the cluster name derived from the service/port specification in the provider config.
Issue: #40027