passing time filters

[alburthoffman]

What is this feature?
refer to jaegertracing/jaeger#4150

The old jaeger /api/traces/:id does not have start and end time parameters, which makes it a global scan request.
Our data volume is super high. And it is very slow to do a global scan.

We had discussed this with jaeger team and the time parameters would be helpful in this case.
The changes happen in Grafana jaeger console instead of jaeger project side, as we implement the API by ourself.
And the APIs are more likely to integrate with Grafana console.

Why do we need this feature?
It's good for those data sources which expose Jaeger APIs, but dont use jaeger in the backend.
The time filters are more flexible.

Who is this feature for?
any implementation which wants to integrate with Grafana Jaeger console

Which issue(s) does this PR fix?:
jaegertracing/jaeger#4150

#64085

Fixes #

Special notes for your reviewer:

[CLAassistant]

All committers have signed the CLA.

[aocenas]

To understand this, you have a custom Jaeger compatible backend that supports the range params, while jaeger itself will just ignore these, right?

I wonder if this is something that we should make opt-in setting in a config page anyway. The reason is whoever uses other backends or if Jeager starts supporting the range params, it may create a situation where users won't be able to easily find their traces because for example in Explore the default time range is last 1h which may be too narrow. So we sort of consciously don't use the range for traces because if the user has a traceID they usually just don't assume they also have to figure out in what time range they want to search for it (traceID being unique and there should be a single instance of it).

[alburthoffman]

what's config page? it would be helpful if you could point me to that.

[alburthoffman]

@aocenas it does not mean time range has to be skipped when "traceID being unique and there should be a single instance of it". What I understand is time range is not required. It can be optional.

The time range is usually there when user finds one trace id from log or exemplar or grafana console or search API. Especially when user open one trace in grafana console, the assumption is the same time range with the searching range.

User should expect slowness when there is no time range, which means global scan in the whole tracing storage. They would expect much faster response with time range info.

Since time range is optional, grafana jaeger datasource can have one option in the setting page to turn this on or off.

[aocenas]

@alburthoffman by config page I mean data source config page for Jaeger where this setting can be configured case by case basis

grafana/public/app/plugins/datasource/jaeger/components/ConfigEditor.tsx

Line 13 in afd39c1

export const ConfigEditor = ({ options, onOptionsChange }: Props) => {

.

The time range is usually there when user finds one trace id from log or exemplar or grafana console or search API. Especially when user open one trace in grafana console, the assumption is the same time range with the searching range.

This is true but it's not the only way people use tracing. Imagine getting trace ID from other sources. You can have a logging tool that does not have data links. You have to copy the trace id into new Explore window and do the search. If you don't also match the exact time range you won't get any results.

At the same time this is not a Jaeger feature. Even if Jaeger just ingores it by default I am reluctant to make use if it as a default state when the reason for being there is IMHO a minority usecase when somebody has a custom Jaeger backend which implements time range params. That is why I would rather make this an optional config that needs to be opted in rather then a default.

[alburthoffman]

@aocenas thx for ur guidance. Working to add a setting for this feature. Thx

[alburthoffman]

@joshhunt @JoaoSilvaGrafana @aocenas please help review the PR. Thx

[alburthoffman]

@aocenas could u take a look? thx

[grafanabot]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[grafanabot]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[tonypowa]

@aocenas
hi Andrej

whenever possible, could you please review the latest commit from the contributor?
thank you

[aocenas]

@tonypowa sorry @adrapereira is handling this now. At the same time this seems like a niche usecase which probably does not make anything else worse but at the same time we are still contemplating whether it makes sense to merge this.