fix(spans): adhere attribute name to otel semver #185
Conversation
| span.SetAttributes(attribute.Int64("http.request_content_length", req.ContentLength), | ||
| attribute.Int("http.request_content_length", 415)) | ||
| span.SetAttributes(semconv.HTTPRequestBodySize(int(req.ContentLength)), | ||
| semconv.HTTPResponseStatusCode(415)) |
There was a problem hiding this comment.
It feels off that the second attribute is the same as the first one. So I set it to HTTPResponseStatusCode, as I expect it to equal the status code that is checked a few lines before this.
| @@ -10,6 +10,7 @@ import ( | |||
| abstractions "github.com/microsoft/kiota-abstractions-go" | |||
| "go.opentelemetry.io/otel" | |||
| "go.opentelemetry.io/otel/attribute" | |||
| semconv "go.opentelemetry.io/otel/semconv/v1.24.0" | |||
There was a problem hiding this comment.
| semconv "go.opentelemetry.io/otel/semconv/v1.24.0" | |
| semconv "go.opentelemetry.io/otel/semconv" |
Should the imports include the version here? Or will they cause issue incase of an update in the go.mod file?
There was a problem hiding this comment.
It did not work when I did not assign the version here. So if you manage to get it working that'd be great :)
There was a problem hiding this comment.
@andrueastman you need to reference the version and yes, it will cause runtime errors if you upgrade the otel sdk and don't have the correct version of semconv referenced. I hace seen multiple issues and proposals for this but no progress I'm aware of to mitigate having to update all references of semconv usage. See open-telemetry/opentelemetry-go#4886
There was a problem hiding this comment.
In general we've faced similar issues in other languages. This dependency causes a lot of issues since they ship breaking changes without major revs (to keep the version aligned with the other packages roughly). We've resulted NOT to use this package in other languages because of that. Yes it introduces misalignment possibilities/added maintenance burden because of that.
I hope this additional context helps!
Updates the http span attributes to match with the updated OTEL specification.
Fixes #182