remove pilot flag PILOT_TRACE_SAMPLING #51792
Conversation
istio-testing
added
the
size/M
| @@ -577,8 +576,7 @@ func configureSampling(hcmTracing *hcm.HttpConnectionManager_Tracing, providerPe | |||
| } | |||
|
|
|||
| func proxyConfigSamplingValue(config *meshconfig.ProxyConfig) float64 { | |||
| // PILOT_TRACE_SAMPLING | |||
| sampling := features.TraceSampling | |||
| sampling := 1.0 | |||
There was a problem hiding this comment.
how can this be turned off? config.Tracing.Sampling != 0.0 { does not allow it
There was a problem hiding this comment.
what do you mean turned off? sampling = 0.0?
There was a problem hiding this comment.
The questions is what is the recommended way to configure trace sampling for pilot. I assumed using the OTel env - https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/ - but this method is confusing me, shouldn't you remove this method and let OTel do its own config ?
what should I do for this? add a deprecation announcement in v1.23 releate notes? |
|
/test integ-ambient-ipv6_istio |
zirain
changed the title
I don't think so, it work with defaultProviders as default value. |
|
Telemetry cannot define default value? |
|
| content: | | ||
| If you are using the pilot environment flag `PILOT_TRACE_SAMPLING`, you should | ||
| replace it with `defaultConfig.tracing.sampling` in the [MeshConfig](https://istio.io/latest/docs/reference/config/istio.mesh.v1alpha1/#ProxyConfig) | ||
| or `randomSamplingPercentage`in [Telemetry API](https://istio.io/latest/docs/reference/config/telemetry/). |
There was a problem hiding this comment.
- I don't see any code using Telemetry API to program Istiod itself.
- TelemetryAPI is based on label selectors - Istiod can be external or run in another managed environment
- The MeshConfig.tracing.sampling is supposed to be replaced by Telemetry API for sidecars - I don't think we should provide multiple options, we already have too many. If you want to use Telemetry API - we should just tell users to use the v1 API, not the alpha legacy one.
- also the needs/expectations for Istiod are quite different from the 'default for all envoys in the mesh'
IMO we should just use the env variables defined by OTel - they are considered stable and that's how users expect to configure any app using the OTel SDK.
Telemetry API would be the right thing to use for HTTP/gRPC straces - if Istiod was behind a Waypoint or Gateway. I am trying to bring back that mode ( I had a very old PR), Istiod is the perfect user of ambient and/or having a Gatweay in front, that would take care of this kind of tracing - but we still have the 'native' tracing using the SDK.
There was a problem hiding this comment.
I don't see any code using Telemetry API to program Istiod itself.
PILOT_TRACE_SAMPLING is not about programming Istiod-internal traces, its about setting the trace sampling for Envoys.
There was a problem hiding this comment.
The name is misleading, I need to pause every time I read this code.
There was a problem hiding this comment.
I don't know why I am even surprised by this after spending days trying to figure out the cert signing interfaces...
Can you put in some comment "for setting pilot tracing, we are using XXX ( or link to some doc - I remember John added OTLP tracing for pilot but forgot how to set it up...)" - and in the release note just mention the v1 telemetry API ( not mesh config )
There was a problem hiding this comment.
Let me try to find out thing about OTLP tracing for pilot(I never know about it).
meshconfig is still useful when users use defaultProviers only(without Telemetry resource).
Please provide a description of this PR:
there's three ways to config trace sampling rate today, it's time to remove pilot flag.