- Notifications
You must be signed in to change notification settings - Fork928
feat: add tracing for sql#1610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
06ca51bf8c3ab793ec818b83917525d5cb13eeda2169ca227af7c5ef46d92172005d4ccc5e0b05b9b654a2ddd774f401e12cb35aaa4c46830f5519bFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -28,14 +28,13 @@ import ( | ||||||||
| "github.com/pion/webrtc/v3" | ||||||||
| "github.com/prometheus/client_golang/prometheus/promhttp" | ||||||||
| "github.com/spf13/cobra" | ||||||||
| sdktrace "go.opentelemetry.io/otel/sdk/trace" | ||||||||
| "golang.org/x/oauth2" | ||||||||
| xgithub "golang.org/x/oauth2/github" | ||||||||
| "golang.org/x/xerrors" | ||||||||
| "google.golang.org/api/idtoken" | ||||||||
| "google.golang.org/api/option" | ||||||||
| "cdr.dev/slog" | ||||||||
| "cdr.dev/slog/sloggers/sloghuman" | ||||||||
| "github.com/coder/coder/cli/cliflag" | ||||||||
| @@ -103,8 +102,11 @@ func server() *cobra.Command { | ||||||||
| logger = logger.Leveled(slog.LevelDebug) | ||||||||
| } | ||||||||
| var ( | ||||||||
| tracerProvider *sdktrace.TracerProvider | ||||||||
| err error | ||||||||
| sqlDriver = "postgres" | ||||||||
| ) | ||||||||
| if trace { | ||||||||
| tracerProvider, err = tracing.TracerProvider(cmd.Context(), "coderd") | ||||||||
| if err != nil { | ||||||||
| @@ -116,6 +118,13 @@ func server() *cobra.Command { | ||||||||
| defer cancel() | ||||||||
| _ = tracerProvider.Shutdown(ctx) | ||||||||
| }() | ||||||||
| d, err := tracing.PostgresDriver(tracerProvider, "coderd.database") | ||||||||
| if err != nil { | ||||||||
| logger.Warn(cmd.Context(), "failed to start postgres tracing driver", slog.Error(err)) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. potential bug: If Suggested change
| ||||||||
| } else { | ||||||||
| sqlDriver = d | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
| @@ -245,7 +254,7 @@ func server() *cobra.Command { | ||||||||
| _, _ = fmt.Fprintln(cmd.ErrOrStderr()) | ||||||||
| if !dev { | ||||||||
| sqlDB, err := sql.Open(sqlDriver, postgresURL) | ||||||||
| if err != nil { | ||||||||
| return xerrors.Errorf("dial postgres: %w", err) | ||||||||
| } | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -6,24 +6,23 @@ import ( | ||
| "github.com/go-chi/chi/middleware" | ||
| "github.com/go-chi/chi/v5" | ||
| sdktrace "go.opentelemetry.io/otel/sdk/trace" | ||
| semconv "go.opentelemetry.io/otel/semconv/v1.10.0" | ||
| ) | ||
| // HTTPMW adds tracing to http routes. | ||
| func HTTPMW(tracerProvider *sdktrace.TracerProvider, name string) func(http.Handler) http.Handler { | ||
| return func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { | ||
| if tracerProvider == nil { | ||
| next.ServeHTTP(rw, r) | ||
| return | ||
| } | ||
| // start span with default span name. Span name will be updated to "method route" format once request finishes. | ||
| ctx, span := tracerProvider.Tracer(name).Start(r.Context(), fmt.Sprintf("%s %s", r.Method, r.RequestURI)) | ||
| defer span.End() | ||
| r = r.WithContext(ctx) | ||
| wrw := middleware.NewWrapResponseWriter(rw, r.ProtoMajor) | ||
| @@ -35,34 +34,21 @@ func HTTPMW(tracerProvider *sdktrace.TracerProvider, name string) func(http.Hand | ||
| if route != "" { | ||
| span.SetName(fmt.Sprintf("%s %s", r.Method, route)) | ||
| } | ||
| span.SetName(fmt.Sprintf("%s %s", r.Method, route)) | ||
| span.SetAttributes(semconv.NetAttributesFromHTTPRequest("tcp", r)...) | ||
| span.SetAttributes(semconv.EndUserAttributesFromHTTPRequest(r)...) | ||
| span.SetAttributes(semconv.HTTPServerAttributesFromHTTPRequest("", route, r)...) | ||
| span.SetAttributes(semconv.HTTPRouteKey.String(route)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Nice change with the | ||
| // set the status code | ||
| status := wrw.Status() | ||
| // 0 status means one has not yet been sent in which case net/http library will write StatusOK | ||
| if status == 0 { | ||
| status = http.StatusOK | ||
| } | ||
| span.SetAttributes(semconv.HTTPStatusCodeKey.Int(status)) | ||
| spanStatus, spanMessage := semconv.SpanStatusFromHTTPStatusCode(status) | ||
| span.SetStatus(spanStatus, spanMessage) | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| package tracing | ||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "strings" | ||
| "github.com/nhatthm/otelsql" | ||
| semconv "go.opentelemetry.io/otel/semconv/v1.7.0" | ||
| "go.opentelemetry.io/otel/trace" | ||
| "golang.org/x/xerrors" | ||
| ) | ||
| // Postgres driver will register a new tracing sql driver and return the driver name. | ||
| func PostgresDriver(tp trace.TracerProvider, service string) (string, error) { | ||
| // Register the otelsql wrapper for the provided postgres driver. | ||
| driverName, err := otelsql.Register("postgres", | ||
| otelsql.WithDefaultAttributes( | ||
| semconv.ServiceNameKey.String(service), | ||
| ), | ||
| otelsql.TraceQueryWithoutArgs(), | ||
| otelsql.WithSystem(semconv.DBSystemPostgreSQL), | ||
| otelsql.WithTracerProvider(tp), | ||
| otelsql.WithSpanNameFormatter(formatPostgresSpan), | ||
| ) | ||
| if err != nil { | ||
| return "", xerrors.Errorf("registering postgres tracing driver: %w", err) | ||
| } | ||
| return driverName, nil | ||
| } | ||
| func formatPostgresSpan(ctx context.Context, op string) string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Should we add the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I'm just copying the format I've seen before with other database tracing tools (New Relic) and also mimicing the format of the http spans. Since an http span is I don't love the prefix because I already can see where the span is coming from by the instrumentation library (see the screenshots in description), but I also don't know if this is the case on other tools like Jaeger, which I'd like to setup eventually. I'm down to change it if we find it hard to parse as we use it, does that sound ok? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Yeah, that sounds perfectly reasonable. 👍🏻 | ||
| const qPrefix = "-- name: " | ||
| q := otelsql.QueryFromContext(ctx) | ||
| if q == "" || !strings.HasPrefix(q, qPrefix) { | ||
| return strings.ToUpper(op) | ||
| } | ||
| // Remove the qPrefix and then grab the method name. | ||
| // We expect the first line of the query to be in | ||
| // the format "-- name: GetAPIKeyByID :one". | ||
| s := strings.SplitN(strings.TrimPrefix(q, qPrefix), " ", 2)[0] | ||
| return fmt.Sprintf("%s %s", strings.ToUpper(op), s) | ||
| } | ||