OpenTelemetry Integration: Refactoring, support configuring OTLP headers #460
Conversation
| // See https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/database/ | ||
| // however note that "db.postgresql.plan" is non-standard. | ||
| span.SetAttributes(attribute.String("db.system", "postgresql")) | ||
| span.SetAttributes(attribute.String("db.postgresql.plan", urlToSample(server, grant, sample))) |
There was a problem hiding this comment.
I don't really follow this change. Neither url.full nor db.postgresql.plan are mentioned in the URL cited from a quick skim, and I don't understand what "plan" means in this context.
There was a problem hiding this comment.
The problem is that there isn't really a proper place to put information like this - my initial thinking was that url.full may be a way to get tracing platforms to actually turn the attribute value into a clickable link, but that does not appear to be the case. It appears url.full is intended to represent the URL that an HTTP call from an application goes to, so not really the same as this use case.
I only today discovered that there was the semantic convention for database calls (and how to represent them in spans), and thus the addition of db.system. I then thought "how could we represent a query plan within this structure", and since there is some existing cases where there is database-system specific data (https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/database/#call-level-attributes-for-specific-technologies) this seemed like the best name.
There was a problem hiding this comment.
Got it, thanks for clarifying.
…pshot This refactors the logic to only initialize the configured tracing provider once for each server, instead of doing so when tracing spans are being sent. Additionally this adds a new verbose log message that indicates when spans were exported successfully.
… plan This changes the span attribute "url.full" to "db.postgresql.plan" to fit better in existing attribute semantics. It also adds the "db.system" attribute set to "postgresql", per the semantic convention for databases: https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/database/
This is necessary for using remote OTLP endpoints provided by providers such as Honeycomb and New Relic, which require the API key to be set through the headers mechanism. Adds the otel_exporter_otlp_headers / OTEL_EXPORTER_OTLP_HEADERS config setting, which follows the regular OpenTelemetry settings format: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/protocol/exporter.md
lfittl
force-pushed
the
support-opentelemetry-headers
branch
from
47c2399
to
da3b04d
Compare
See individual commits.