Support quoted attribute in traceql #3004

kousikmitra · 2023-10-11T06:24:17Z

What this PR does:
This PR extends the traceql lexer to lex quoted attribute names (ex: span."foo bar") this will help searching tempo with span attributes containing breaking characters like =, " etc.

Which issue(s) this PR fixes:
Fixes #2805

Checklist

Tests updated
Documentation added
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

knylander-grafana · 2023-10-12T19:12:59Z

docs/sources/tempo/traceql/_index.md

+It's possible that attributes name can contain terminal characters, to search span attributes with terminal character you can use quoted attribute syntax. A quoted attirbute should be enclosed inside double quote and all characters between the quotes will be considered part of the attribute name.
+
+For example, to find span with attribute name `attribute name with space` one can use the following query


Thank you for adding documentation! These suggestions correct a few grammatical issues and separate run-on sentence. For more information, refer to the Writers' Toolkit.

Suggested change

It's possible that attributes name can contain terminal characters, to search span attributes with terminal character you can use quoted attribute syntax. A quoted attirbute should be enclosed inside double quote and all characters between the quotes will be considered part of the attribute name.

For example, to find span with attribute name `attribute name with space` one can use the following query

Attribute names can contain terminal characters, such as a period (`.`). To search span attributes with terminal characters, you can use quoted attribute syntax. A quoted attribute should be enclosed inside double quotes, for example, `"example one"`. All characters between the quotes are considered part of the attribute name.

For example, to find span with attribute name `attribute name with space`, you can use the following query:

Thanks a lot. 🙂 I have made the changes and will refer to the mentioned style guide going forward.

docs/sources/tempo/traceql/_index.md

knylander-grafana · 2023-10-12T20:17:59Z

docs/sources/tempo/traceql/_index.md

+{ span.attribute."attribute name with space" = "value" }
+```
+
+Note: Only `\"` and `\\` escape sequence are supported as of now.


This change uses the shortcode for a Note admonition. Using shortcodes helps us separate content from presentation. To learn more about the shortcode, refer to the Writers' Toolkit.

Suggested change

Note: Only `\"` and `\\` escape sequence are supported as of now.

{{% admonition type="note" %}}

Currently, only `\"` and `\\` escape sequence are supported.

{{% /admonition %}}

knylander-grafana · 2023-10-12T20:19:38Z

CHANGELOG.md

@@ -1,4 +1,5 @@
 ## main / unreleased
+* [ENHANCEMENT] Support quoted attribute name in traceql [#3004](https://github.com/grafana/tempo/pull/3004) (@kousikmitra)


Correcting capitalization for TraceQL.

Suggested change

* [ENHANCEMENT] Support quoted attribute name in traceql [#3004](https://github.com/grafana/tempo/pull/3004) (@kousikmitra)

* [ENHANCEMENT] Support quoted attribute name in TraceQL [#3004](https://github.com/grafana/tempo/pull/3004) (@kousikmitra)

knylander-grafana

Thank you for updating the doc! I've made some suggestions to help clarify the content you added. Please let me know if you have any questions.

joe-elliott

nice work! had some thoughts and questions

pkg/traceql/parse_test.go

pkg/traceql/lexer.go

joe-elliott

Looks great! One last request. Can we get a test added at this level to really confirm everything is working as intended:

https://github.com/grafana/tempo/blob/main/tempodb/tempodb_search_test.go#L73

I linked the test that made the most sense to me, but if you want to put it in a different runner or add a new one you are welcome to.

pkg/traceql/parse_test.go

joe-elliott · 2023-10-16T12:06:36Z

pkg/traceql/lexer.go

+	return sb.String(), nil
+}
+
+func tryScopeAtribute(l *scanner.Scanner) (int, bool) {


Suggested change

func tryScopeAtribute(l *scanner.Scanner) (int, bool) {

func tryScopeAttribute(l *scanner.Scanner) (int, bool) {

jdbaldry

@knylander-grafana We might want to expand on the explanation of escape sequences. WDYT?

I've proposed some adjustments to the docs to fix some typos and remove some extraneous words.

jdbaldry · 2023-10-17T10:37:38Z

docs/sources/tempo/traceql/_index.md

@@ -119,6 +119,26 @@ For example, to find traces with an attribute of `sla` set to `critical`:
 { .sla = "critical" }
 ```

+### Quoted attribute names
+
+Attribute names can contain terminal characters, such as a period (`.`). To search span attributes with terminal characters, you can use quoted attribute syntax. A quoted attribute should be enclosed inside double quotes, for example, `"example one"`. All characters between the quotes are considered part of the attribute name.


nit: Semantic line breaks are better for future line based diffing in the GitHub UI and result in the same HTML output.

Suggested change

Attribute names can contain terminal characters, such as a period (`.`). To search span attributes with terminal characters, you can use quoted attribute syntax. A quoted attribute should be enclosed inside double quotes, for example, `"example one"`. All characters between the quotes are considered part of the attribute name.

Attribute names can contain terminal characters, such as a period (`.`).

To search span attributes with terminal characters, you can use quoted attribute syntax.

A quoted attribute should be enclosed inside double quotes, for example, `"example one"`.

All characters between the quotes are considered part of the attribute name.

jdbaldry · 2023-10-17T10:39:07Z

docs/sources/tempo/traceql/_index.md

+For example, to find span with attribute name `attribute name with space`, you can use the following query:
+```
+{ ."attribute name with space" = "value" }
+```


Suggested change

For example, to find span with attribute name `attribute name with space`, you can use the following query:

```

{ ."attribute name with space" = "value" }

```

To find a span with the attribute name `attribute name with space`, use the following query:

```

{ ."attribute name with space" = "value" }

```

jdbaldry · 2023-10-17T10:39:54Z

docs/sources/tempo/traceql/_index.md

+You can use quoted attributes syntax with non-quoted attribute syntax.
+
+For example, the syntax below is a valid TraceQL query.
+```
+{ span.attribute."attribute name with space" = "value" }
+```


Suggested change

You can use quoted attributes syntax with non-quoted attribute syntax.

For example, the syntax below is a valid TraceQL query.

```

{ span.attribute."attribute name with space" = "value" }

```

You can use quoted attributes syntax with non-quoted attribute syntax, the following is a valid TraceQL query:

```

{ span.attribute."attribute name with space" = "value" }

```

jdbaldry · 2023-10-17T10:40:21Z

docs/sources/tempo/traceql/_index.md

+```
+
+{{% admonition type="note" %}}
+Currently, only `\"` and `\\` escape sequence are supported.


Suggested change

Currently, only `\"` and `\\` escape sequence are supported.

Currently, only the `\"` and `\\` escape sequences are supported.

Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com>

kousikmitra · 2023-10-17T14:31:06Z

@joe-elliott @jdbaldry Made the requested changes. Let me know if this is ok.

tempodb/tempodb_search_test.go

joe-elliott

Great work!

kousikmitra requested review from joe-elliott, annanay25, mdisibio, mapno, kvrhdn, zalegrala, electron0zero, ie-pham and stoewer as code owners October 11, 2023 06:24

kousikmitra requested a review from knylander-grafana as a code owner October 12, 2023 13:17

knylander-grafana reviewed Oct 12, 2023

View reviewed changes

docs/sources/tempo/traceql/_index.md Outdated Show resolved Hide resolved

knylander-grafana reviewed Oct 12, 2023

View reviewed changes

joe-elliott reviewed Oct 12, 2023

View reviewed changes

pkg/traceql/parse_test.go Show resolved Hide resolved

pkg/traceql/parse_test.go Show resolved Hide resolved

pkg/traceql/parse_test.go Show resolved Hide resolved

pkg/traceql/lexer.go Outdated Show resolved Hide resolved

pkg/traceql/lexer.go Outdated Show resolved Hide resolved

kousikmitra force-pushed the feature/quoted-attribute branch from a5ba22c to 3024f63 Compare October 13, 2023 07:40

kousikmitra requested a review from jdbaldry as a code owner October 14, 2023 06:24

kousikmitra requested review from joe-elliott and knylander-grafana October 14, 2023 06:27

joe-elliott reviewed Oct 16, 2023

View reviewed changes

jdbaldry reviewed Oct 17, 2023

View reviewed changes

kousikmitra and others added 8 commits October 17, 2023 19:57

Support quoted attribute in traceql

4c77e3a

Add documentation for quoted attribute syntax

be09b36

Add changelog entry

fc08ccf

Add doc review change

ec54066

Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com>

Refactor scope parsing

4e1004a

Refactor attribute lexing part

ce5fb25

Add test for non terminated quoted attribute

2b8676d

Fix typo

359e5ed

kousikmitra added 2 commits October 17, 2023 19:59

Add doc review changes

bbfd8d7

Add tests for quoted attributes

44c9ca1

kousikmitra force-pushed the feature/quoted-attribute branch from ebdd21a to 44c9ca1 Compare October 17, 2023 14:30

kousikmitra commented Oct 17, 2023

View reviewed changes

tempodb/tempodb_search_test.go Outdated Show resolved Hide resolved

Update tempodb testcase

717f1dc

joe-elliott approved these changes Oct 17, 2023

View reviewed changes

joe-elliott merged commit bdac8e9 into grafana:main Oct 17, 2023
15 checks passed

kousikmitra deleted the feature/quoted-attribute branch October 17, 2023 18:45

fabrizio-grafana mentioned this pull request Oct 27, 2023

Update TraceQL query editor's grammar according to last changes to Tempo's TraceQL grafana/grafana#77288

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support quoted attribute in traceql #3004

Support quoted attribute in traceql #3004

kousikmitra commented Oct 11, 2023 •

edited

Loading

knylander-grafana Oct 12, 2023 •

edited

Loading

kousikmitra Oct 13, 2023

knylander-grafana Oct 12, 2023

knylander-grafana Oct 12, 2023

knylander-grafana left a comment

joe-elliott left a comment

joe-elliott left a comment

joe-elliott Oct 16, 2023

jdbaldry left a comment

jdbaldry Oct 17, 2023

jdbaldry Oct 17, 2023

jdbaldry Oct 17, 2023

jdbaldry Oct 17, 2023

kousikmitra commented Oct 17, 2023

joe-elliott left a comment

		It's possible that attributes name can contain terminal characters, to search span attributes with terminal character you can use quoted attribute syntax. A quoted attirbute should be enclosed inside double quote and all characters between the quotes will be considered part of the attribute name.

		For example, to find span with attribute name `attribute name with space` one can use the following query

		@@ -1,4 +1,5 @@
		## main / unreleased
		* [ENHANCEMENT] Support quoted attribute name in traceql [#3004](https://github.com/grafana/tempo/pull/3004) (@kousikmitra)

	func tryScopeAtribute(l *scanner.Scanner) (int, bool) {
	func tryScopeAttribute(l *scanner.Scanner) (int, bool) {

	Currently, only `\"` and `\\` escape sequence are supported.
	Currently, only the `\"` and `\\` escape sequences are supported.

Support quoted attribute in traceql #3004

Support quoted attribute in traceql #3004

Conversation

kousikmitra commented Oct 11, 2023 • edited Loading

knylander-grafana Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knylander-grafana left a comment

Choose a reason for hiding this comment

joe-elliott left a comment

Choose a reason for hiding this comment

joe-elliott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdbaldry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kousikmitra commented Oct 17, 2023

joe-elliott left a comment

Choose a reason for hiding this comment

kousikmitra commented Oct 11, 2023 •

edited

Loading

knylander-grafana Oct 12, 2023 •

edited

Loading