-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Use all span kinds as default for metrics #3898
Use all span kinds as default for metrics #3898
Conversation
Instead of just using "SPAN_KIND_SERVER" as the default span kind for the metrics, the default value now includes all span kinds. Background: The span kind for some services could have another value then "SPAN_KIND_SERVER", e.g. the spans from Istio are using "SPAN_KIND_CLIENT". Because of the default span kind and the fact that the span kind is not configureable in the frontend (the query parameter is never set in the API calls) it was not possible to use the service performance monitoring with some services. Signed-off-by: ricoberger <mail@ricoberger.de>
@ricoberger that would create a lot of noise IMHO in the metrics expected in SPM. Say, 1 server call has downstream 1000 calls to other dbs, queues and other services. With the suggested approach, all the spans will be used to calculate metrics for that 1 server call. |
The jaeger-query API allows callers to override the span kind. I agree that there are situations where it makes sense to use a span kind other than IMO, the more correct place to configure the span kind is in Jaeger UI; either configured on startup or exposed in the UI as a way to override the default |
Closing this PR for now. Feel free to re-open if you wish to discuss further. |
Hi, sorry I completely forgot about this. It's definitely ok to close the PR we tested this further during the last weeks and implemented a solution in our Jaeger frontend, where the user can select the span kind. |
Nice one @ricoberger! Would you be open to contributing what you've done to the /~https://github.com/jaegertracing/jaeger-ui/ project? I think it's a valid use case to query on non-server span kinds and would be valuable to others in the community. |
@albertteoh |
## Which problem is this PR solving? - Resolves jaegertracing/jaeger#4035 - Related to jaegertracing/jaeger#3898 ## Description of the changes - Add a span kind filter to the Monitor tab. - As span kinds are an attribute of a span, much like the service name, I thought it made sense to add the dropdown filter next to the service filter. - I'm not a designer so open to ideas on how to make this better! ## How was this change tested? - Manually tested as per the following screen recording. - Update tests. /~https://github.com/jaegertracing/jaeger-ui/assets/26584478/2d8ee49f-e30c-4438-9e92-9012814c495d ## Checklist - [x] I have read /~https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: albertteoh <see.kwang.teoh@gmail.com> Signed-off-by: Albert Teoh <albert@packsmith.io> Co-authored-by: Albert Teoh <albert@packsmith.io>
Hi everyone, if this change doesn't make sense or if I missed something to configure the span kind feel free to close the PR.
Which problem is this PR solving?
Background: The span kind for some services could have another value then "SPAN_KIND_SERVER", e.g. the spans from Istio are using "SPAN_KIND_CLIENT". Because of the default span kind and the fact that the span kind is not configureable in the frontend (the query parameter is never set in the API calls) it was not possible to use the service performance monitoring with some services.
Short description of the changes
Instead of just using "SPAN_KIND_SERVER" as the default span kind for the metrics, the default value now includes all span kinds.