Tempo: Integrate scoped tags API #68106
Conversation
joey-grafana
added
area/frontend
add to changelog
area/datasource/frontend
datasource/Tempo
no-backport
| * We expect the tags list data directly from the request and assign it an empty set here. | ||
| */ | ||
| setTags(tags: string[]) { | ||
| tags.forEach((t) => (this.tags[t] = new Set<string>())); |
There was a problem hiding this comment.
Not sure why we were setting each tag to an empty set before. Guessing it's from an older implementation that has since been updated
| let v1Resp, v2Resp; | ||
| try { | ||
| v2Resp = await this.request('/api/v2/search/tags', []); | ||
| } catch (error) { | ||
| v1Resp = await this.request('/api/search/tags', []); | ||
| } | ||
| this.tags = { | ||
| v1: v2Resp ? undefined : v1Resp.tagNames, | ||
| v2: v2Resp && v2Resp.scopes ? v2Resp.scopes : undefined, | ||
| }; |
There was a problem hiding this comment.
The idea here is to support users who are using a version of Tempo with the new scoped tags API /api/v2/search/tags and also users who are using a version of Tempo with the older API /api/search/tags
| provideCompletionItems = async ({ text, value }: TypeaheadInput): Promise<TypeaheadOutput> => { | ||
| const emptyResult: TypeaheadOutput = { suggestions: [] }; | ||
|
|
||
| if (!value) { | ||
| return emptyResult; | ||
| } | ||
|
|
||
| const query = value.endText.getText(); | ||
| const isValue = query[query.indexOf(text) - 1] === '='; | ||
| if (isValue || text === '=') { | ||
| return this.getTagValueCompletionItems(value); | ||
| } | ||
| return this.getTagsCompletionItems(); | ||
| }; | ||
|
|
||
| getTagsCompletionItems = (): TypeaheadOutput => { | ||
| const { tags } = this; | ||
| const suggestions: CompletionItemGroup[] = []; | ||
|
|
||
| if (tags?.length) { | ||
| suggestions.push({ | ||
| label: `Tag`, | ||
| items: tags.map((tag) => ({ label: tag })), | ||
| }); | ||
| } | ||
|
|
||
| return { suggestions }; | ||
| }; | ||
|
|
||
| async getTagValueCompletionItems(value: Value) { | ||
| const tags = value.endText.getText().split(' '); | ||
|
|
||
| let tagName = tags[tags.length - 1] ?? ''; | ||
| tagName = tagName.split('=')[0]; | ||
|
|
||
| const response = await this.request(`/api/v2/search/tag/${tagName}/values`, []); | ||
|
|
||
| const suggestions: CompletionItemGroup[] = []; | ||
|
|
||
| if (response && response.tagValues) { | ||
| suggestions.push({ | ||
| label: `Tag Values`, | ||
| items: response.tagValues.map((tagValue: string) => ({ label: tagValue, insertText: `"${tagValue}"` })), | ||
| }); | ||
| } | ||
| return { suggestions }; | ||
| } | ||
|
|
There was a problem hiding this comment.
This code is no longer used.
| export const getFilteredTags = (tags: Tags | undefined, staticTags: Array<string | undefined>) => { | ||
| let filteredTags; | ||
| if (tags) { | ||
| filteredTags = { ...tags }; | ||
| if (tags.v1) { | ||
| filteredTags.v1 = [...intrinsics, ...tags.v1].filter((t) => !staticTags.includes(t)); | ||
| } else if (tags.v2) { | ||
| filteredTags.v2 = tags.v2.map((scope: Scope) => { | ||
| if (scope.name && scope.name !== 'intrinsic') { | ||
| return { | ||
| ...scope, | ||
| tags: scope.tags ? [...intrinsics, ...scope.tags].filter((t) => !staticTags.includes(t)) : [], | ||
| }; | ||
| } | ||
| return { ...scope }; | ||
| }); | ||
| } | ||
| } | ||
| return filteredTags; | ||
| }; |
There was a problem hiding this comment.
I'm not sure about us handling the v2 and v1 fallback manually across the code base. Ideally we would change Tags to be a class, instantiated with the values of either v1 or v2, with public functions to retrieve the values as needed. Consumers of Tags shouldn't have to worry about the fallback, just that they have a list of tags.
For example:
export class Tags {
v1: string[] | undefined;
v2: Scope[] | undefined;
constructor(private v1?: string[], private v2?: Scope[]) {
...
}
getTags(scope?: string): string[] | undefined {
// take care of all the logic here and fallback to v1 as needed
}
}You can still keep this getFilteredTags function as an util, but it should receive a list of strings instead of a Tags object so it doesn't have to account for the fallback to v1.
Our goal with this change is to reduce duplicated code, prevent future bugs by mishandling or forgetting the fallback, make the code easier to read and centralise the fallback logic.
There was a problem hiding this comment.
Ahh yes this is a great suggestion :)
There was a problem hiding this comment.
I pushed changes!
| let tags: string[] = []; | ||
| if (this.tags) { | ||
| if (this.tags.v1) { | ||
| tags = this.tags.v1; | ||
| } else if (this.tags.v2) { | ||
| const s = this.tags.v2.find((s: Scope) => s.name && s.name === scope); | ||
| // have not typed a scope yet || unscoped (.) typed | ||
| if (!s) { | ||
| tags = getUnscopedTags(this.tags.v2); | ||
| } else { | ||
| tags = s && s.tags ? s.tags : []; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return tags |
There was a problem hiding this comment.
Same as other comment, this logic should be moved to the Tags class ideally.
This block of code should be replaced by return this.tags.getTags(scope)....
I leave the getTags function name up to you, it can be renamed to something else.
What is this feature?
Integrates the new scoped tags API.
Why do we need this feature?
Allows the TraceQL autocomplete + TraceQLSearch (search) tab + static filters to show only the correct tags when searching by span and resource scopes. Prior to the introduction of this API we showed users all tags, regardless of the scope selected.
Who is this feature for?
Tempo users.
Which issue(s) does this PR fix?:
Fixes #67011
Special notes for your reviewer:
Please check that: