ariga / sqlcomment

sqlcommenter support for ent
Apache License 2.0
56 stars 3 forks source link

Clarify differences between sqlcomment and otelsql #4

Open StevenACoffman opened 2 years ago

StevenACoffman commented 2 years ago

I read that Sqlcommenter merged into OpenTelemetry.

I have database connection code that I have already instrumented with https://github.com/XSAM/otelsql

    wrappedDriverName, err := otelsql.Register(
        "pgx",
        semconv.DBSystemPostgreSQL.Value.AsString(),
    )

and I am wondering if adding sqlcomment is redundant or if it would provide additional value?

giautm commented 2 years ago

Is redundant or if it would provide additional value?

I think it provides additional value to track where the request comes from, from apiv1, GraphQL, or gRPC functions. The merge in Google's term applied to the spec, the trace collector, and the cloud service.

For client instruction, it's fine if we have two separate packages to provide the functions.

Link the issue: https://github.com/XSAM/otelsql/issues/109

rotemtam commented 2 years ago

hey @StevenACoffman thanks for reaching out. I have self-assigned the issue and will respond soon

rotemtam commented 2 years ago

hey @StevenACoffman!

Sqlcommenter is a standard created at Google and recently accepted into OpenTelemetry. It's an interesting addition, and I'm curious to see how the integration will take place. In general, with otel telematics are exported as either traces (spans) or as metrics. Below these abstractions, you will find technologies like Jaeger (for tracing) or Prometheus (for metrics).

Sqlcommenter works quite differently. It works on the ORM layer, by injecting metadata containing comments as part of the SQL statements that are sent to the database. These logs are then ingested on the database side (with support from Cloud SQL Insights, a proprietary Google Cloud offering). I am not aware of OSS solutions that do this ingestion and expose this data to the user. yet, but I'm sure with this becoming part of otel we will begin seeing something like that.

To answer your question:

HTH

StevenACoffman commented 2 years ago

Thanks! That's very helpful. I was hoping to get sqlcomment to add the de facto standardized fields traceparent and tracestate. For example:

select * from widgets /*traceparent='00-5bd66ef5095369c7b0d1f8f4bd33716a-c532cb4098ac3dd2-01',tracestate='congo%3Dt61rcWkgMzE%2Crojo%3D00f067aa0ba902b7'*/

I'd like to be able to use both Cloud SQL Insights and Cloud Trace in my application.

Does ariga/sqlcomment and NewOTELTagger() do that?

StevenACoffman commented 2 years ago

Oh, nevermind, it looks like it does just that: https://github.com/ariga/sqlcomment/blob/6bb67a62a61af6abbcb732734036dfe965a5e0e8/otel.go#L17