pkg

pkg copied to clipboard

This is still WIP (commented out a number of unused methods, will cleanup).

Like the log change, there are a few small updates in istio repo, but very small. Almost all of Istio is using the float variables and the methods removed are not used and will simplify migration from OC.

😊 Welcome @costinm! This is either your first contribution to the Istio pkg repo, or it's been awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@costinm: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
gencheck_pkg	2bb7ef98ea9f78156f9c9a0a8372d6bedb3c8d7e	link	true	/test gencheck
lint_pkg	2bb7ef98ea9f78156f9c9a0a8372d6bedb3c8d7e	link	true	/test lint
test_pkg	2bb7ef98ea9f78156f9c9a0a8372d6bedb3c8d7e	link	true	/test test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

OTel metrics are not yet 'stable' - while prom is a dependency we already have. Even with otel API, exporting to prometheus is still based on the prometheus client library, by registering a 'generator'.

We can still use otel traces which are v1+ - but in practice registering a counter/gauge/historgram with the prometheus client library will continue to work even if we start registering some counters with OTel API when it moves to v1 - and we have an opportunity to validate the performance and impact.

We can also start using OTel collector - which supports reading prom metrics and pushing to OTLP endpoints.

On Mon, May 8, 2023 at 12:09 PM John Howard @.***> wrote:

@.**** commented on this pull request.

In monitoring/monitoring_opencensus.go https://github.com/istio/pkg/pull/804#discussion_r1187795436:

@@ -0,0 +1,229 @@ +//go:build !skip_opencensus

Why prom? I thought we were moving to otel?

— Reply to this email directly, view it on GitHub https://github.com/istio/pkg/pull/804#discussion_r1187795436, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUR2XC7DMA6S7AFE3D3M3XFFAHPANCNFSM6AAAAAAX2HZ2AE . You are receiving this because you were mentioned.Message ID: @.***>

Few reasons:

1. Otel metric SDK is not yet 1.0/stable
2. "Stated goal" may or may not be equivalent with reality - we have a lot of stated goals too
3. It is an interface - best to have multiple implementations, so we can actually compare for ourselves.
4. Not clear if Istio should take the burden of configuring other exporters (and their auth) instead of letting OTel-collector or similar tools handle that - across k8s including non-istio workloads. Same applies to istiod.

When Otel metric is stable/1.0 and we check the numbers - we may as well start using the OTel API directly instead of a wrapper ( I think we should do the same for log once new slog is available ).

On Tue, May 9, 2023 at 1:45 AM Kuat @.***> wrote:

@.**** commented on this pull request.

In monitoring/monitoring_opencensus.go https://github.com/istio/pkg/pull/804#discussion_r1187907756:

@@ -0,0 +1,229 @@ +//go:build !skip_opencensus

Why not use otel SDK to allow exporters besides prometheus? Semantically they should be equivalent per their stated goal.

— Reply to this email directly, view it on GitHub https://github.com/istio/pkg/pull/804#discussion_r1187907756, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUR2XM63ZOPL6SGD6H3Z3XFH737ANCNFSM6AAAAAAX2HZ2AE . You are receiving this because you were mentioned.Message ID: @.***>

SGTM, as long as we maintain a wrapper and don't force any library until Otel is ready.

Agreed, just like we want Istio users to only use stable features in production and not experimental - we should do the same process for our dependencies :-)

And even when OTel metrics is v1 - we would still want to test and compare it with the prom library which is in broad use. Whatever is faster and better can be used ( unless prometheus library gets deprecated ).

OTel tracing is v1 - and we can use it, and I believe for access logs there is some gray area. There is also a subtle difference between protocol and the client libraries - supporting OTLP and OTel is possible without using a particular library.

On Tue, May 9, 2023 at 9:09 AM Kuat @.***> wrote:

SGTM, as long as we maintain a wrapper and don't force any library until Otel is ready.

— Reply to this email directly, view it on GitHub https://github.com/istio/pkg/pull/804#issuecomment-1540473475, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUR2QLDZAI3K4TWN6EFFLXFJT43ANCNFSM6AAAAAAX2HZ2AE . You are receiving this because you were mentioned.Message ID: @.***>