frontend: add more attributes to the spans #1322
Conversation
simonpasquier
force-pushed
the
add-more-span-attributes
branch
from
c776528 to
478b6ec
Compare
| @@ -72,13 +64,8 @@ func TestMiddlewareTracing(t *testing.T) { | |||
| } { | |||
| t.Run(tc.name, func(t *testing.T) { | |||
| // Setup the testing tracer. | |||
| inMemoryExporter := tracetest.NewInMemoryExporter() | |||
There was a problem hiding this comment.
fyi @frzifus I've reworked a bit the test framework for tracing. let me know what you think.
simonpasquier
requested review from
bennerv,
mbarnes,
SudoBrendan,
mociarain,
venkateshsredhat,
Nanyte25,
kostola and
hbhushan3
as code owners
simonpasquier
force-pushed
the
add-more-span-attributes
branch
from
478b6ec to
58e632f
Compare
|
|
||
| var attrs []attribute.KeyValue | ||
| if a.subscriptionID != "" { | ||
| attrs = append(attrs, attribute.String("aro.subscription_id", a.subscriptionID)) |
There was a problem hiding this comment.
Are we sticking to subscription_id or subcription.id? I can't remember if we had a conversion on the ID convention or not. Remind me if we did.
There was a problem hiding this comment.
good point and I don't remember that we decided explicitly for one or the other.
There was a problem hiding this comment.
but since we already have aro.correlation.id, it's probably best to stick with the '.' approach. I'll update.
There was a problem hiding this comment.
ok after thinking a bit more, I have taken a mixed approach. We can discuss more next week (nothing urgent here).
simonpasquier
force-pushed
the
add-more-span-attributes
branch
2 times, most recently
from
c1b0ebe to
10a7824
Compare
This change modifies the existing middlewares to add new span attributes and be consistent with logging. In practice, the following attributes are added: * `cloud.provider` (always `Azure`) [1] * `cloud.resource_id` [1] * `aro.subscription.id` * `aro.subscription.state` * `aro.resource_group.name` * `aro.resource.name` * `aro.api_version` The following attributes have also been renamed to better match the guidelines on attribute naming (TL;DR: use dot separators to delimit namespaces and underscores for multi-word names): * `aro.correlation.id` -> `aro.correlation_id` * `aro.client.request.id` -> `aro.client.request_id` * `aro.request.id` -> `aro.request_id` [1] https://pkg.go.dev/go.opentelemetry.io/otel@v1.34.0/semconv/v1.27.0 Signed-off-by: Simon Pasquier <spasquie@redhat.com>
simonpasquier
force-pushed
the
add-more-span-attributes
branch
from
10a7824 to
907c7e1
Compare
|
Everything looks fine for now. Will sync up on the trace IDs next week to move this this PR forward. |
What this PR does
This change modifies the existing middlewares to add span attributes and be consistent with logging.
The following attributes are added:
cloud.provider(alwaysAzure) [1]cloud.resource_id[1]aro.subscription.idaro.subscription.statearo.resource_group.namearo.resource.namearo.api_versionThe following attributes have also been renamed to better match the
guidelines on attribute naming (TL;DR: use dot separators to delimit
namespaces and underscores for multi-word names):
aro.correlation.id->aro.correlation_idaro.client.request.id->aro.client.request_idaro.request.id->aro.request_id[1] https://pkg.go.dev/go.opentelemetry.io/otel@v1.34.0/semconv/v1.27.0
Jira:
Link to demo recording:
Special notes for your reviewer