Skip to content

Commit

Permalink
database: revert to official github.com/XSAM/otelsql (sourcegraph#47429)
Browse files Browse the repository at this point in the history
This is enabled by sourcegraph#47428
- swapping `SpanProcessor` instead of `TracerProvider` means that we can
correctly propagate configuration changes to `Tracer` instances, so that
we no longer need XSAM/otelsql#115 and can
revert back to upstream. Switching to upstream, which has moved
considerably since we forked it, necessitates a few other
OpenTelemetry-related dependency upgrades as well (split out from
sourcegraph#47126)

Whether updates are work is tested in
sourcegraph#47428

## Test plan

<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->

Search with `?trace=1` still has SQL traces with args:

<img width="1649" alt="Screenshot 2023-02-07 at 4 27 57 PM"
src="https://user-images.githubusercontent.com/23356519/217288396-3069e947-1841-4108-8ab6-02a384a98a54.png">

<img width="1649" alt="Screenshot 2023-02-07 at 4 27 57 PM"
src="https://user-images.githubusercontent.com/23356519/217288612-f5091180-a2f3-4144-ab6f-3456cc187612.png">
  • Loading branch information
bobheadxi committed Feb 21, 2023
1 parent afb3a5c commit a071a07
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 22 deletions.
11 changes: 4 additions & 7 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ require (
github.com/NYTimes/gziphandler v1.1.1
github.com/PuerkitoBio/rehttp v1.1.0
github.com/RoaringBitmap/roaring v1.2.3
github.com/XSAM/otelsql v0.18.0
github.com/agext/levenshtein v1.2.3
github.com/amit7itz/goset v1.0.1
github.com/aws/aws-sdk-go-v2 v1.17.4
Expand Down Expand Up @@ -186,8 +187,6 @@ require (
sigs.k8s.io/yaml v1.3.0
)

require github.com/XSAM/otelsql v0.15.0

require (
cloud.google.com/go/compute/metadata v0.2.3 // indirect
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.14.1 // indirect
Expand Down Expand Up @@ -223,7 +222,7 @@ require (
github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect
github.com/yusufpapurcu/wmi v1.2.2 // indirect
github.com/zenazn/goji v1.0.1 // indirect
go.opentelemetry.io/otel/metric v0.34.0 // indirect
go.opentelemetry.io/otel/metric v0.35.0 // indirect
go.uber.org/goleak v1.2.0 // indirect
)

Expand Down Expand Up @@ -455,8 +454,8 @@ require (
go.mongodb.org/mongo-driver v1.10.2 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/collector/pdata v0.56.0 // indirect
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.37.0
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.36.4
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.38.0
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.38.0
go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.13.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.13.0
go.opentelemetry.io/proto/otlp v0.19.0
Expand Down Expand Up @@ -502,8 +501,6 @@ replace (
// These entries indicate temporary replace directives due to a pending pull request upstream
// or issues with specific versions.
replace (
// Forked until PR is merged upstream TODO @jhchabran
github.com/XSAM/otelsql => github.com/sourcegraph/otelsql v0.0.0-20220905085252-74375c884fff
// Pending: https://github.com/crewjam/saml/pull/450
github.com/crewjam/saml => github.com/sourcegraph/saml v0.0.0-20220728002234-ab6b53f6f94d
// Pending: https://github.com/ghodss/yaml/pull/65
Expand Down
17 changes: 9 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ github.com/Shopify/logrus-bugsnag v0.0.0-20171204204709-577dee27f20d/go.mod h1:H
github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo=
github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI=
github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg=
github.com/XSAM/otelsql v0.18.0 h1:n9PcapiW6W801A4QgYG2ArfY2JwnWduXorRlZqucVhM=
github.com/XSAM/otelsql v0.18.0/go.mod h1:M0w5PR/rB7GdEBiHsJ9C91NEKkD2uWFcKj+l5xj8K+c=
github.com/acomagu/bufpipe v1.0.3 h1:fxAGrHZTgQ9w5QqVItgzwj235/uYZYgbXitB+dLupOk=
github.com/acomagu/bufpipe v1.0.3/go.mod h1:mxdxdup/WdsKVreO5GpW4+M/1CE2sMG4jeGJ2sYmHc4=
github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo=
Expand Down Expand Up @@ -2098,8 +2100,6 @@ github.com/sourcegraph/mountinfo v0.0.0-20230106004439-7026e28cef67 h1:NSYSPQOE7
github.com/sourcegraph/mountinfo v0.0.0-20230106004439-7026e28cef67/go.mod h1:4DAabK408OEbyK2NUEQ5YRApyB/p0XNGJyC1YPBAKq4=
github.com/sourcegraph/oauth2 v0.0.0-20210825125341-77c1d99ece3c h1:HGa4iJr6MGKnB5qbU7tI511NdGuHUHnNCqP67G6KmfE=
github.com/sourcegraph/oauth2 v0.0.0-20210825125341-77c1d99ece3c/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
github.com/sourcegraph/otelsql v0.0.0-20220905085252-74375c884fff h1:u/Lf5xLBDYfRjyeGk8+zUqXrWwRzMIwFbhqKPIWo79Q=
github.com/sourcegraph/otelsql v0.0.0-20220905085252-74375c884fff/go.mod h1:DpO7NCSeqQdr23nU0yapjR3jGx2OdO/PihPRG+/PV0Y=
github.com/sourcegraph/run v0.12.0 h1:3A8w5e8HIYPfafHekvmdmmh42RHKGVhmiTZAPJclg7I=
github.com/sourcegraph/run v0.12.0/go.mod h1:PwaP936BTnAJC1cqR5rSbG5kOs/EWStTK3lqvMX5GUA=
github.com/sourcegraph/saml v0.0.0-20220728002234-ab6b53f6f94d h1:S9aS/W4oJ5gSUJuTv94Gurm/3vh/qJATjfkEUrrnszU=
Expand Down Expand Up @@ -2367,12 +2367,12 @@ go.opentelemetry.io/collector/pdata v0.56.0/go.mod h1:mYcCREWiIJyHss0dbU+GSiz2tm
go.opentelemetry.io/collector/semconv v0.56.0 h1:zpQ6IBimBsiVsJibsSM2/13vKtaeteFFIx4bmIiOS6E=
go.opentelemetry.io/contrib v0.21.0/go.mod h1:EH4yDYeNoaTqn/8yCWQmfNB78VHfGX2Jt2bvnvzBlGM=
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.21.0/go.mod h1:Vm5u/mtkj1OMhtao0v+BGo2LUoLCgHYXvRmj0jWITlE=
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.37.0 h1:+uFejS4DCfNH6d3xODVIGsdhzgzhh45p9gpbHQMbdZI=
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.37.0/go.mod h1:HSmzQvagH8pS2/xrK7ScWsk0vAMtRTGbMFgInXCi8Tc=
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.38.0 h1:g/BAN5o90Pr6D8xMRezjzGOHBpc15U+4oE53nZLiae4=
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.38.0/go.mod h1:+F41JBSkye7aYJELRvIMF0Z66reIwIOL0St75ZVwSJs=
go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0.21.0/go.mod h1:a9cocRplhIBkUAJmak+BPDx+LVL7cTmqUPB0uBcTA4k=
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.21.0/go.mod h1:JQAtechjxLEL81EjmbRwxBq/XEzGaHcsPuDHAx54hg4=
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.36.4 h1:aUEBEdCa6iamGzg6fuYxDA8ThxvOG240mAvWDU+XLio=
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.36.4/go.mod h1:l2MdsbKTocpPS5nQZscqTR9jd8u96VYZdcpF8Sye7mA=
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.38.0 h1:rTxmym+VN9f6ajzNtITVgyvZrNbpLt3NHr3suLLHLEQ=
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.38.0/go.mod h1:w6xNm+kC506KNs5cknSHal6dtdRnc4uema0uN9GSQc0=
go.opentelemetry.io/contrib/propagators/jaeger v1.14.0 h1:j6Xah53xRDrR+K1c4Y1uVHA0ESo69xDOblw+3OrVoF4=
go.opentelemetry.io/contrib/propagators/jaeger v1.14.0/go.mod h1:viOfwr1OqHmCF6G3KvKnnmpSJUX/rLzXztU18FC9ymU=
go.opentelemetry.io/contrib/propagators/ot v1.14.0 h1:jqxznjMkz/3l4NUsYq4OMbP+zs5twBbCZwSlSt82KXo=
Expand All @@ -2398,12 +2398,13 @@ go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.13.0 h1:Ntu7i
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.13.0/go.mod h1:wZ9SAjm2sjw3vStBhlCfMZWZusyOQrwrHOFo00jyMC4=
go.opentelemetry.io/otel/internal/metric v0.21.0/go.mod h1:iOfAaY2YycsXfYD4kaRSbLx2LKmfpKObWBEv9QK5zFo=
go.opentelemetry.io/otel/metric v0.21.0/go.mod h1:JWCt1bjivC4iCrz/aCrM1GSw+ZcvY44KCbaeeRhzHnc=
go.opentelemetry.io/otel/metric v0.34.0 h1:MCPoQxcg/26EuuJwpYN1mZTeCYAUGx8ABxfW07YkjP8=
go.opentelemetry.io/otel/metric v0.34.0/go.mod h1:ZFuI4yQGNCupurTXCwkeD/zHBt+C2bR7bw5JqUm/AP8=
go.opentelemetry.io/otel/metric v0.35.0 h1:aPT5jk/w7F9zW51L7WgRqNKDElBdyRLGuBtI5MX34e8=
go.opentelemetry.io/otel/metric v0.35.0/go.mod h1:qAcbhaTRFU6uG8QM7dDo7XvFsWcugziq/5YI065TokQ=
go.opentelemetry.io/otel/oteltest v1.0.0-RC1/go.mod h1:+eoIG0gdEOaPNftuy1YScLr1Gb4mL/9lpDkZ0JjMRq4=
go.opentelemetry.io/otel/sdk v1.0.0-RC1/go.mod h1:kj6yPn7Pgt5ByRuwesbaWcRLA+V7BSDg3Hf8xRvsvf8=
go.opentelemetry.io/otel/sdk v1.13.0 h1:BHib5g8MvdqS65yo2vV1s6Le42Hm6rrw08qU6yz5JaM=
go.opentelemetry.io/otel/sdk v1.13.0/go.mod h1:YLKPx5+6Vx/o1TCUYYs+bpymtkmazOMT6zoRrC7AQ7I=
go.opentelemetry.io/otel/sdk/metric v0.35.0 h1:gryV4W5GzpOhKK48/lZb8ldyWIs3DRugSVlQZmCwELA=
go.opentelemetry.io/otel/trace v1.0.0-RC1/go.mod h1:86UHmyHWFEtWjfWPSbu0+d0Pf9Q6e1U+3ViBOc+NXAg=
go.opentelemetry.io/otel/trace v1.13.0 h1:CBgRZ6ntv+Amuj1jDsMhZtlAPT6gbyIRdaIzFhfBSdY=
go.opentelemetry.io/otel/trace v1.13.0/go.mod h1:muCvmmO9KKpvuXSf3KKAXXB2ygNYHQ+ZfI5X08d3tds=
Expand Down
28 changes: 21 additions & 7 deletions internal/database/dbconn/open.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"database/sql/driver"
"fmt"
"log"
"strconv"
"strings"
Expand All @@ -17,6 +18,7 @@ import (
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/qustavo/sqlhooks/v2"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"

"github.com/sourcegraph/sourcegraph/internal/env"
"github.com/sourcegraph/sourcegraph/lib/errors"
Expand Down Expand Up @@ -257,14 +259,8 @@ func open(cfg *pgx.ConnConfig) (*sql.DB, error) {
OmitConnPrepare: true,
OmitRows: true,
OmitConnectorConnect: true,
ArgumentOptions: otelsql.ArgumentOptions{
EnableAttributes: true,
Skip: func(ctx context.Context, query string, args []any) bool {
// Do not decorate span with args as attributes if that's a bulk insertion
// or if we have too many args (it's unreadable anyway).
return isBulkInsertion(ctx) || len(args) > 24
}},
}),
otelsql.WithAttributesGetter(argsAsAttributes),
)
if err != nil {
return nil, errors.Wrap(err, "postgresql open")
Expand Down Expand Up @@ -303,3 +299,21 @@ func isDatabaseLikelyStartingUp(err error) bool {

return false
}

// argsAsAttributes generates a set of OpenTelemetry trace attributes that represent the
// argument values used in a query.
func argsAsAttributes(ctx context.Context, _ otelsql.Method, _ string, args []driver.NamedValue) []attribute.KeyValue {
// Do not decorate span with args as attributes if that's a bulk insertion
// or if we have too many args (it's unreadable anyway).
if isBulkInsertion(ctx) || len(args) > 24 {
return []attribute.KeyValue{attribute.Bool("db.args.skipped", true)}
}

attrs := make([]attribute.KeyValue, len(args))
for i, arg := range args {
attrs[i] = attribute.String(
fmt.Sprintf("db.args.$%d", arg.Ordinal),
fmt.Sprintf("%v", arg.Value))
}
return attrs
}

0 comments on commit a071a07

Please sign in to comment.