-
Notifications
You must be signed in to change notification settings - Fork 559
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
Promote Telemetry API to v1 #3133
Conversation
3dbc901
to
f1b99e4
Compare
@zirain @lei-tang @kyessenov what is your thought on this? Do you feel confident it should be v1 API now? |
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.
I think a doc listing the tests, the user guides, the usage data for each API field to promote will help reviewers to evaluate its v1 readiness. We probably do not want to promote untested/undocumented/rarely used API fields. It is engineering costly to maintain a v1 API field.
Signed-off-by: whitneygriffith <whitney.griffith16@gmail.com>
e4d71a8
to
4cab11a
Compare
Signed-off-by: whitneygriffith <whitney.griffith16@gmail.com>
4cab11a
to
e524823
Compare
Signed-off-by: whitneygriffith <whitney.griffith16@gmail.com>
LGTM Thanks for driving |
LGTM as well, although I thought I had put a comment in the PR previously. Thanks. |
Sorry, just seeing it. As I always bring it up - I'm not sure you can claim backwards compatibility for things that are not provided in Envoy. CEL and access log formatter syntax are not part of xDS API and are not subject to compatibility expectations, so that's not something that Istio can provide. I'm also not sure what other extensions are needed by this API and whether they are mature enough. |
Access log formatter or labels are not part of the v1 API.
CEL on the other hand - and the metric label opt-in/out - are part of the
API, I had my doubts as well but it looks like the use cases are strong and
replacing it wasn't feasible.
xDS API and Envoy may change - but the Istio project is committed to
support the v1 API. If Envoy makes changes or lacks a feature - it will
be our responsibility to implement Istio-specific extensions or fix Envoy
and continue to support the API for our users.
Good news is that (AFAIK) the v1 API does not extend to ambient - in
ambient L4 and L7 are swappable and are not
required to be OSS Istio-Envoy based. I think that implies that telemetry
API and set of metrics may not be based on Istio,
but on whatever Gateway or OTel define.
…On Mon, Apr 8, 2024 at 2:03 PM Kuat ***@***.***> wrote:
Sorry, just seeing it. As I always bring it up - I'm not sure you can
claim backwards compatibility for things that are not provided in Envoy.
CEL and access log formatter syntax are not part of xDS API and are not
subject to compatibility expectations, so that's not something that Istio
can provide. I'm also not sure what other extensions are needed by this API
and whether they are mature enough.
—
Reply to this email directly, view it on GitHub
<#3133 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUR2W562N6AQNIBPI2OK3Y4MA3PAVCNFSM6AAAAABE4OZS2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBTGYZTQMRYGY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
IMO: Swappable waypoints does not imply that Istio APIs change. An implementation can chose to implement the Istio waypoints or not. Probably we would have some conformance to say they are "Base Waypoint" conformant vs "Istio API Waypoint" conformant. |
On Tue, Apr 9, 2024 at 7:55 AM John Howard ***@***.***> wrote:
IMO: Swappable waypoints does not imply that Istio APIs change. An
implementation can chose to implement the Istio waypoints or not. Probably
we would have some conformance to say they are "Base Waypoint" conformant
vs "Istio API Waypoint" conformant.
Istio API does change between '????' and 'ambient' - even if the same CRDs
are used, the fields, usage and behavior are different,
workload selector versus target, client overrides, etc. ( don't know how
we're calling the 'original Istio API for sidecars with workload selectors
and all the features promoted to v1')
Telemetry in particular is quite different.
For Waypoints that implement the Gateway API - I agree passing the
Gateway/GAMMA conformance would be very important,
and they should list which 'vendor extensions' they support. I don't know
if it's reasonable to expect a non-Istiod Waypoint to
support Istio APIs, but if it happens they would include it in their docs,
including what subset of fields they do ( since the
CRDs we have are a mix of fields ). Do you have any expectations that
other, non Istiod-based implementations ( i.e. not
forks of Istio) will suddenly adopt Istio API ? That was the belief 5 years
ago, that other meshes and gateways will adopt our
API and telemetry.
We should discuss it today, around API versioning - each field in the CRD
that applies to ambient should be marked
somehow, as well as the docs need some update for the behaviors that
change. Telemetry v1 for example
has no mention of how it behaves in ambient or for K8S Gateways/GAMMA.
—
… Reply to this email directly, view it on GitHub
<#3133 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUR2RO75QLMZLKMYHQ5HLY4P6M5AVCNFSM6AAAAABE4OZS2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBVGM4DOOBRGQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Resolves 46720 and #155
Telemetry Features not promoted:
These features are annotated with
// +cue-gen:Telemetry:releaseChannel:standard
which will be functional with the release channel work #173See Telemetry API RFC