-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Remove OpenCensus tracer from Istio XDS #51078
Remove OpenCensus tracer from Istio XDS #51078
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
@@ -200,14 +195,6 @@ func configureFromProviderConfig(pushCtx *model.PushContext, proxy *model.Proxy, | |||
} | |||
return otelLightStepConfig(clusterName, hostname, provider.Lightstep.GetAccessToken()) | |||
} | |||
case *meshconfig.MeshConfig_ExtensionProvider_Opencensus: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably add some form of warnings if these are used.
Maybe in manifests/charts/istio-control/istio-discovery/templates/NOTES.txt for helm and operator/pkg/apis/istio/v1alpha1/validation/validation.go for operator?
can be a followup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a warn log is adoptable.
"@type": "type.googleapis.com/envoy.config.trace.v3.OpenCensusConfig", | ||
"stackdriver_exporter_enabled": true, | ||
"stackdriver_project_id": "{{ .stackdriverProjectID }}", | ||
{{ if .sts_port }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we only use sts_port for stackdriver? If so, can clean up that code in istio-agent as well in the future
cc @kyessenov
(note: the STS I cleaned up was separate, we had 2 STS things in the agent)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's different impersonations I believe. Yeah, we can move it to Google agent as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
STS is also used for Google CA. STS clean up can be a separate work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Google CA's usage of STS is in #51043
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@howardjohn Can you review/approve this PR? Thanks! |
Please provide a description of this PR:
OpenCensus is deprecated. This PR removes the OpenCensus tracer from Istio XDS.
Related issue: #50808.
To help us figure out who should review this PR, please put an X in all the areas that this PR affects.