From 07949df530407cee37ee48537fc04b831c5f36b1 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Mon, 26 Jan 2026 17:07:32 -0600 Subject: [PATCH] Improved OpenTelemetry error handling (#38757) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Related issue:** Resolves #38756 - Changed to NOT mark many client errors as exceptions - Instead, added client_error and server_error metrics that can be used to alert on unusual error rates # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## Summary by CodeRabbit * **New Features** * Added separate metrics for distinguishing between client and server errors, enhancing observability and monitoring capabilities. * **Bug Fixes** * Client request errors no longer incorrectly appear in error tracking as exceptions; improved accuracy of error reporting to external services. * Adjusted logging levels for authentication and enrollment operations to provide clearer diagnostics. ✏️ Tip: You can customize this high-level summary in your review settings. --- changes/38756-otel-client-errors | 1 + cmd/fleet/serve.go | 27 ++++- go.mod | 30 ++--- go.sum | 76 +++++++------ server/contexts/ctxerr/ctxerr.go | 131 ++++++++++++++-------- server/contexts/ctxerr/ctxerr_test.go | 46 ++++++++ server/contexts/ctxerr/metrics.go | 56 +++++++++ server/datastore/mysql/mysql.go | 4 +- server/fleet/errors.go | 42 +++++++ server/platform/http/errors.go | 32 ++++++ server/platform/mysql/errors.go | 5 + server/service/integration_logger_test.go | 6 +- server/service/osquery.go | 2 +- server/service/transport_error.go | 13 +++ 14 files changed, 362 insertions(+), 109 deletions(-) create mode 100644 changes/38756-otel-client-errors create mode 100644 server/contexts/ctxerr/metrics.go diff --git a/changes/38756-otel-client-errors b/changes/38756-otel-client-errors new file mode 100644 index 0000000000..ee7ba6d618 --- /dev/null +++ b/changes/38756-otel-client-errors @@ -0,0 +1 @@ +Improved OpenTelemetry error handling: client errors (4xx) no longer set span status to Error or appear in the Exceptions tab, following OTEL semantic conventions. Added separate metrics for client vs server errors (`fleet.http.client_errors`, `fleet.http.server_errors`) with error type attribution. Client errors are also no longer sent to APM/Sentry. diff --git a/cmd/fleet/serve.go b/cmd/fleet/serve.go index 3e745aebe8..9d2f1b0800 100644 --- a/cmd/fleet/serve.go +++ b/cmd/fleet/serve.go @@ -93,11 +93,13 @@ import ( _ "go.elastic.co/apm/module/apmsql/v2" _ "go.elastic.co/apm/module/apmsql/v2/mysql" "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" "go.opentelemetry.io/otel/exporters/otlp/otlptrace" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" - semconv "go.opentelemetry.io/otel/semconv/v1.34.0" + semconv "go.opentelemetry.io/otel/semconv/v1.37.0" "google.golang.org/grpc" _ "google.golang.org/grpc/encoding/gzip" // Because we use gzip compression for OTLP ) @@ -160,8 +162,9 @@ the way that the Fleet server works. createTestBuckets(&config, logger) } - // Init tracing + // Init tracing and metrics var tracerProvider *sdktrace.TracerProvider + var meterProvider *sdkmetric.MeterProvider if config.Logging.TracingEnabled { ctx := context.Background() client := otlptracegrpc.NewClient( @@ -193,6 +196,19 @@ the way that the Fleet server works. sdktrace.WithSpanProcessor(batchSpanProcessor), ) otel.SetTracerProvider(tracerProvider) + + // Initialize OTEL metrics exporter + metricExporter, err := otlpmetricgrpc.New(ctx, + otlpmetricgrpc.WithCompressor("gzip"), + ) + if err != nil { + initFatal(err, "Failed to initialize OTEL metrics exporter") + } + meterProvider = sdkmetric.NewMeterProvider( + sdkmetric.WithResource(res), + sdkmetric.WithReader(sdkmetric.NewPeriodicReader(metricExporter)), + ) + otel.SetMeterProvider(meterProvider) } allowedHostIdentifiers := map[string]bool{ @@ -1665,12 +1681,17 @@ the way that the Fleet server works. cancelFunc() cleanupCronStatsOnShutdown(ctx, ds, logger, instanceID) launcher.GracefulStop() - // Flush any pending OTEL spans before shutting down + // Flush any pending OTEL data before shutting down if tracerProvider != nil { if err := tracerProvider.Shutdown(ctx); err != nil { level.Error(logger).Log("msg", "failed to shutdown OTEL tracer provider", "err", err) } } + if meterProvider != nil { + if err := meterProvider.Shutdown(ctx); err != nil { + level.Error(logger).Log("msg", "failed to shutdown OTEL meter provider", "err", err) + } + } return srv.Shutdown(ctx) }() }() diff --git a/go.mod b/go.mod index 5b0b6193b7..0a24b0aab6 100644 --- a/go.mod +++ b/go.mod @@ -149,11 +149,14 @@ require ( go.etcd.io/bbolt v1.3.10 go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux v0.60.0 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 - go.opentelemetry.io/otel v1.38.0 - go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.37.0 - go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.37.0 - go.opentelemetry.io/otel/sdk v1.37.0 - go.opentelemetry.io/otel/trace v1.38.0 + go.opentelemetry.io/otel v1.39.0 + go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.39.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.39.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.39.0 + go.opentelemetry.io/otel/metric v1.39.0 + go.opentelemetry.io/otel/sdk v1.39.0 + go.opentelemetry.io/otel/sdk/metric v1.39.0 + go.opentelemetry.io/otel/trace v1.39.0 golang.org/x/crypto v0.45.0 golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 golang.org/x/image v0.18.0 @@ -161,12 +164,12 @@ require ( golang.org/x/net v0.47.0 golang.org/x/oauth2 v0.33.0 golang.org/x/sync v0.18.0 - golang.org/x/sys v0.38.0 + golang.org/x/sys v0.39.0 golang.org/x/term v0.37.0 golang.org/x/text v0.31.0 golang.org/x/tools v0.38.0 google.golang.org/api v0.256.0 - google.golang.org/grpc v1.76.0 + google.golang.org/grpc v1.77.0 gopkg.in/guregu/null.v3 v3.5.0 gopkg.in/ini.v1 v1.67.0 gopkg.in/natefinch/lumberjack.v2 v2.0.0 @@ -215,7 +218,7 @@ require ( github.com/beorn7/perks v1.0.1 // indirect github.com/c-bata/go-prompt v0.2.3 // indirect github.com/cavaliergopher/cpio v1.0.1 // indirect - github.com/cenkalti/backoff/v5 v5.0.2 // indirect + github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/cespare/xxhash v1.1.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/cloudflare/circl v1.6.1 // indirect @@ -266,7 +269,7 @@ require ( github.com/goreleaser/chglog v0.4.2 // indirect github.com/goreleaser/fileglob v1.3.0 // indirect github.com/gorilla/schema v1.4.1 // indirect - github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.1 // indirect + github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-hclog v1.6.3 // indirect github.com/hashicorp/go-version v1.7.0 // indirect @@ -343,15 +346,14 @@ require ( go.elastic.co/fastjson v1.1.0 // indirect go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352 // indirect go.opencensus.io v0.24.0 // indirect - go.opentelemetry.io/auto/sdk v1.1.0 // indirect + go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0 // indirect - go.opentelemetry.io/otel/metric v1.38.0 // indirect - go.opentelemetry.io/proto/otlp v1.7.0 // indirect + go.opentelemetry.io/proto/otlp v1.9.0 // indirect go.uber.org/multierr v1.11.0 // indirect golang.org/x/time v0.14.0 // indirect google.golang.org/genproto v0.0.0-20250603155806-513f23925822 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20250804133106-a7a43d27e69b // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20251103181224-f26f9409b101 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20251202230838-ff82c1b0f217 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20251202230838-ff82c1b0f217 // indirect google.golang.org/protobuf v1.36.10 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index d88b1edf90..9f3b0862bd 100644 --- a/go.sum +++ b/go.sum @@ -196,8 +196,8 @@ github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEe github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= -github.com/cenkalti/backoff/v5 v5.0.2 h1:rIfFVxEf1QsI7E1ZHfp/B4DF/6QBAUhmgkxc0H7Zss8= -github.com/cenkalti/backoff/v5 v5.0.2/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw= +github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM= +github.com/cenkalti/backoff/v5 v5.0.3/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= @@ -210,8 +210,8 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk github.com/cloudflare/circl v1.6.1 h1:zqIqSPIndyBh1bjLVVDHMPpVKqp8Su/V+6MeDzzQBQ0= github.com/cloudflare/circl v1.6.1/go.mod h1:uddAzsPgqdMAYatqJ0lsjX1oECcQLIlRpzZh3pJrofs= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= -github.com/cncf/xds/go v0.0.0-20250501225837-2ac532fd4443 h1:aQ3y1lwWyqYPiWZThqv1aFbZMiM9vblcSArJRf2Irls= -github.com/cncf/xds/go v0.0.0-20250501225837-2ac532fd4443/go.mod h1:W+zGtBO5Y1IgJhy4+A9GOqVhqLpfZi+vwmdNXUehLA8= +github.com/cncf/xds/go v0.0.0-20251022180443-0feb69152e9f h1:Y8xYupdHxryycyPlc9Y+bSQAYZnetRJ70VMVKm5CKI0= +github.com/cncf/xds/go v0.0.0-20251022180443-0feb69152e9f/go.mod h1:HlzOvOjVBOfTGSRXRyY0OiCS/3J1akRGQQpRO/7zyF4= github.com/containerd/cgroups v1.1.0 h1:v8rEWFl6EoqHB+swVNjVoCJE8o3jX7e8nqBGPLaDFBM= github.com/containerd/cgroups v1.1.0/go.mod h1:6ppBcbh/NOOUU+dMKrykgaBnK9lCIBxHqJDGwsa1mIw= github.com/containerd/containerd v1.7.29 h1:90fWABQsaN9mJhGkoVnuzEY+o1XDPbg9BTC9QTAHnuE= @@ -309,9 +309,9 @@ github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FM github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= -github.com/envoyproxy/go-control-plane v0.13.4 h1:zEqyPVyku6IvWCFwux4x9RxkLOMUL+1vC9xUFv5l2/M= -github.com/envoyproxy/go-control-plane/envoy v1.32.4 h1:jb83lalDRZSpPWW2Z7Mck/8kXZ5CQAFYVjQcdVIr83A= -github.com/envoyproxy/go-control-plane/envoy v1.32.4/go.mod h1:Gzjc5k8JcJswLjAx1Zm+wSYE20UrLtt7JZMWiWQXQEw= +github.com/envoyproxy/go-control-plane v0.13.5-0.20251024222203-75eaa193e329 h1:K+fnvUM0VZ7ZFJf0n4L/BRlnsb9pL/GuDG6FqaH+PwM= +github.com/envoyproxy/go-control-plane/envoy v1.35.0 h1:ixjkELDE+ru6idPxcHLj8LBVc2bFP7iBytj353BoHUo= +github.com/envoyproxy/go-control-plane/envoy v1.35.0/go.mod h1:09qwbGVuSWWAyN5t/b3iyVfz5+z8QWGrzkoqm/8SbEs= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v1.2.1 h1:DEo3O99U8j4hBFwbJfrz9VtgcDfUKS7KJ7spH3d86P8= github.com/envoyproxy/protoc-gen-validate v1.2.1/go.mod h1:d/C80l/jxXLdfEIhX1W2TmLfsJ31lvEjwamM4DxlWXU= @@ -507,8 +507,8 @@ github.com/groob/finalizer v0.0.0-20170707115354-4c2ed49aabda h1:5ikpG9mYCMFiZX0 github.com/groob/finalizer v0.0.0-20170707115354-4c2ed49aabda/go.mod h1:MyndkAZd5rUMdNogn35MWXBX1UiBigrU8eTj8DoAC2c= github.com/groob/plist v0.0.0-20220217120414-63fa881b19a5 h1:saaSiB25B1wgaxrshQhurfPKUGJ4It3OxNJUy0rdOjU= github.com/groob/plist v0.0.0-20220217120414-63fa881b19a5/go.mod h1:itkABA+w2cw7x5nYUS/pLRef6ludkZKOigbROmCTaFw= -github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.1 h1:X5VWvz21y3gzm9Nw/kaUeku/1+uBhcekkmy4IkffJww= -github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.1/go.mod h1:Zanoh4+gvIgluNqcfMVTJueD4wSS5hT7zTt4Mrutd90= +github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 h1:NmZ1PKzSTQbuGHw9DGPFomqkkLWMC+vZCkfs+FHv1Vg= +github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3/go.mod h1:zQrxl1YP88HQlA6i9c63DSVPFklWpGX4OWAc9bFuaH4= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= @@ -760,8 +760,8 @@ github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUc github.com/rogpeppe/fastuuid v1.1.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/rogpeppe/go-internal v1.8.0/go.mod h1:WmiCO8CzOY8rg0OYDC4/i/2WRWAB6poM+XZ2dLUbcbE= -github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= -github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= +github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= +github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.32.0 h1:keLypqrlIjaFsbmJOBdB/qvyF8KEtCWHwobLp5l/mQ0= github.com/rs/zerolog v1.32.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= @@ -939,32 +939,34 @@ go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352/go.mod h1:SNgMg+EgDFwmvS go.opencensus.io v0.22.1/go.mod h1:Ap50jQcDJrx6rB6VgeeFPtuPIf3wMRvRfrfYDO6+BmA= go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0= go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo= -go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA= -go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A= +go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= +go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux v0.60.0 h1:iLuogsToNW6QaOYPcbIwhkdRTkc0gvXzuiajObXc6WY= go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux v0.60.0/go.mod h1:XNSNQBtSOifFUw0aQUyBN0Ff+0NddEnbSATy2QlFgm8= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0 h1:q4XOmH/0opmeuJtPsbFNivyl7bCt7yRBbeEm2sC/XtQ= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0/go.mod h1:snMWehoOh2wsEwnvvwtDyFCxVeDAODenXHtn5vzrKjo= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 h1:F7Jx+6hwnZ41NSFTO5q4LYDtJRXBf2PD0rNBkeB/lus= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0/go.mod h1:UHB22Z8QsdRDrnAtX4PntOl36ajSxcdUMt1sF7Y6E7Q= -go.opentelemetry.io/otel v1.38.0 h1:RkfdswUDRimDg0m2Az18RKOsnI8UDzppJAtj01/Ymk8= -go.opentelemetry.io/otel v1.38.0/go.mod h1:zcmtmQ1+YmQM9wrNsTGV/q/uyusom3P8RxwExxkZhjM= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.37.0 h1:Ahq7pZmv87yiyn3jeFz/LekZmPLLdKejuO3NcK9MssM= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.37.0/go.mod h1:MJTqhM0im3mRLw1i8uGHnCvUEeS7VwRyxlLC78PA18M= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.37.0 h1:EtFWSnwW9hGObjkIdmlnWSydO+Qs8OwzfzXLUPg4xOc= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.37.0/go.mod h1:QjUEoiGCPkvFZ/MjK6ZZfNOS6mfVEVKYE99dFhuN2LI= +go.opentelemetry.io/otel v1.39.0 h1:8yPrr/S0ND9QEfTfdP9V+SiwT4E0G7Y5MO7p85nis48= +go.opentelemetry.io/otel v1.39.0/go.mod h1:kLlFTywNWrFyEdH0oj2xK0bFYZtHRYUdv1NklR/tgc8= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.39.0 h1:cEf8jF6WbuGQWUVcqgyWtTR0kOOAWY1DYZ+UhvdmQPw= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.39.0/go.mod h1:k1lzV5n5U3HkGvTCJHraTAGJ7MqsgL1wrGwTj1Isfiw= +go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.39.0 h1:f0cb2XPmrqn4XMy9PNliTgRKJgS5WcL/u0/WRYGz4t0= +go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.39.0/go.mod h1:vnakAaFckOMiMtOIhFI2MNH4FYrZzXCYxmb1LlhoGz8= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.39.0 h1:in9O8ESIOlwJAEGTkkf34DesGRAc/Pn8qJ7k3r/42LM= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.39.0/go.mod h1:Rp0EXBm5tfnv0WL+ARyO/PHBEaEAT8UUHQ6AGJcSq6c= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.35.0 h1:xJ2qHD0C1BeYVTLLR9sX12+Qb95kfeD/byKj6Ky1pXg= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.35.0/go.mod h1:u5BF1xyjstDowA1R5QAO9JHzqK+ublenEW/dyqTjBVk= -go.opentelemetry.io/otel/metric v1.38.0 h1:Kl6lzIYGAh5M159u9NgiRkmoMKjvbsKtYRwgfrA6WpA= -go.opentelemetry.io/otel/metric v1.38.0/go.mod h1:kB5n/QoRM8YwmUahxvI3bO34eVtQf2i4utNVLr9gEmI= -go.opentelemetry.io/otel/sdk v1.37.0 h1:ItB0QUqnjesGRvNcmAcU0LyvkVyGJ2xftD29bWdDvKI= -go.opentelemetry.io/otel/sdk v1.37.0/go.mod h1:VredYzxUvuo2q3WRcDnKDjbdvmO0sCzOvVAiY+yUkAg= -go.opentelemetry.io/otel/sdk/metric v1.37.0 h1:90lI228XrB9jCMuSdA0673aubgRobVZFhbjxHHspCPc= -go.opentelemetry.io/otel/sdk/metric v1.37.0/go.mod h1:cNen4ZWfiD37l5NhS+Keb5RXVWZWpRE+9WyVCpbo5ps= -go.opentelemetry.io/otel/trace v1.38.0 h1:Fxk5bKrDZJUH+AMyyIXGcFAPah0oRcT+LuNtJrmcNLE= -go.opentelemetry.io/otel/trace v1.38.0/go.mod h1:j1P9ivuFsTceSWe1oY+EeW3sc+Pp42sO++GHkg4wwhs= -go.opentelemetry.io/proto/otlp v1.7.0 h1:jX1VolD6nHuFzOYso2E73H85i92Mv8JQYk0K9vz09os= -go.opentelemetry.io/proto/otlp v1.7.0/go.mod h1:fSKjH6YJ7HDlwzltzyMj036AJ3ejJLCgCSHGj4efDDo= +go.opentelemetry.io/otel/metric v1.39.0 h1:d1UzonvEZriVfpNKEVmHXbdf909uGTOQjA0HF0Ls5Q0= +go.opentelemetry.io/otel/metric v1.39.0/go.mod h1:jrZSWL33sD7bBxg1xjrqyDjnuzTUB0x1nBERXd7Ftcs= +go.opentelemetry.io/otel/sdk v1.39.0 h1:nMLYcjVsvdui1B/4FRkwjzoRVsMK8uL/cj0OyhKzt18= +go.opentelemetry.io/otel/sdk v1.39.0/go.mod h1:vDojkC4/jsTJsE+kh+LXYQlbL8CgrEcwmt1ENZszdJE= +go.opentelemetry.io/otel/sdk/metric v1.39.0 h1:cXMVVFVgsIf2YL6QkRF4Urbr/aMInf+2WKg+sEJTtB8= +go.opentelemetry.io/otel/sdk/metric v1.39.0/go.mod h1:xq9HEVH7qeX69/JnwEfp6fVq5wosJsY1mt4lLfYdVew= +go.opentelemetry.io/otel/trace v1.39.0 h1:2d2vfpEDmCJ5zVYz7ijaJdOF59xLomrvj7bjt6/qCJI= +go.opentelemetry.io/otel/trace v1.39.0/go.mod h1:88w4/PnZSazkGzz/w84VHpQafiU4EtqqlVdxWy+rNOA= +go.opentelemetry.io/proto/otlp v1.9.0 h1:l706jCMITVouPOqEnii2fIAuO3IVGBRPV5ICjceRb/A= +go.opentelemetry.io/proto/otlp v1.9.0/go.mod h1:xE+Cx5E/eEHw+ISFkwPLwCZefwVjY+pqKg1qcK03+/4= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= @@ -1078,8 +1080,8 @@ golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= -golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= +golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -1128,18 +1130,18 @@ google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98 google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo= google.golang.org/genproto v0.0.0-20250603155806-513f23925822 h1:rHWScKit0gvAPuOnu87KpaYtjK5zBMLcULh7gxkCXu4= google.golang.org/genproto v0.0.0-20250603155806-513f23925822/go.mod h1:HubltRL7rMh0LfnQPkMH4NPDFEWp0jw3vixw7jEM53s= -google.golang.org/genproto/googleapis/api v0.0.0-20250804133106-a7a43d27e69b h1:ULiyYQ0FdsJhwwZUwbaXpZF5yUE3h+RA+gxvBu37ucc= -google.golang.org/genproto/googleapis/api v0.0.0-20250804133106-a7a43d27e69b/go.mod h1:oDOGiMSXHL4sDTJvFvIB9nRQCGdLP1o/iVaqQK8zB+M= -google.golang.org/genproto/googleapis/rpc v0.0.0-20251103181224-f26f9409b101 h1:tRPGkdGHuewF4UisLzzHHr1spKw92qLM98nIzxbC0wY= -google.golang.org/genproto/googleapis/rpc v0.0.0-20251103181224-f26f9409b101/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk= +google.golang.org/genproto/googleapis/api v0.0.0-20251202230838-ff82c1b0f217 h1:fCvbg86sFXwdrl5LgVcTEvNC+2txB5mgROGmRL5mrls= +google.golang.org/genproto/googleapis/api v0.0.0-20251202230838-ff82c1b0f217/go.mod h1:+rXWjjaukWZun3mLfjmVnQi18E1AsFbDN9QdJ5YXLto= +google.golang.org/genproto/googleapis/rpc v0.0.0-20251202230838-ff82c1b0f217 h1:gRkg/vSppuSQoDjxyiGfN4Upv/h/DQmIR10ZU8dh4Ww= +google.golang.org/genproto/googleapis/rpc v0.0.0-20251202230838-ff82c1b0f217/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY= google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc= -google.golang.org/grpc v1.76.0 h1:UnVkv1+uMLYXoIz6o7chp59WfQUYA2ex/BXQ9rHZu7A= -google.golang.org/grpc v1.76.0/go.mod h1:Ju12QI8M6iQJtbcsV+awF5a4hfJMLi4X0JLo94ULZ6c= +google.golang.org/grpc v1.77.0 h1:wVVY6/8cGA6vvffn+wWK5ToddbgdU3d8MNENr4evgXM= +google.golang.org/grpc v1.77.0/go.mod h1:z0BY1iVj0q8E1uSQCjL9cppRj+gnZjzDnzV0dHhrNig= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/server/contexts/ctxerr/ctxerr.go b/server/contexts/ctxerr/ctxerr.go index e3526fdcfd..1e5da73f66 100644 --- a/server/contexts/ctxerr/ctxerr.go +++ b/server/contexts/ctxerr/ctxerr.go @@ -309,60 +309,75 @@ func Handle(ctx context.Context, err error) { // Collect telemetry context from registered providers telemetryAttrs := collectTelemetryContext(ctx) - // send to OpenTelemetry if there's an active span - if span := trace.SpanFromContext(ctx); span != nil && span.IsRecording() { - // Mark the current span as failed by setting the error status. - // This status can be overridden if we recovered from the error. - exceptionType := fmt.Sprintf("%T", Cause(cause)) // type of root error - span.SetStatus(codes.Error, exceptionType) // low-cardinality identifier + // Check if this is a client error. Per OTEL semantic conventions, + // 4xx errors on server spans MUST NOT set span status to Error. + // See: https://opentelemetry.io/docs/specs/semconv/http/http-spans/ + clientErr := isClientError(err) + exceptionType := fmt.Sprintf("%T", Cause(cause)) // type of root error - // Build attributes for the exception event - attrs := []attribute.KeyValue{ - attribute.String("exception.type", exceptionType), - attribute.String("exception.message", cause.Error()), - attribute.String("exception.stacktrace", strings.Join(cause.Stack(), "\n")), - } - - // Add contextual information from telemetry providers. - // OpenTelemetry requires typed attributes, so we convert the values to the appropriate type. - for k, v := range telemetryAttrs { - switch val := v.(type) { - case string: - attrs = append(attrs, attribute.String(k, val)) - case int: - attrs = append(attrs, attribute.Int64(k, int64(val))) - case int64: - attrs = append(attrs, attribute.Int64(k, val)) - case uint: - attrs = append(attrs, attribute.Int64(k, int64(val))) //nolint:gosec - case uint64: - attrs = append(attrs, attribute.Int64(k, int64(val))) //nolint:gosec - case bool: - attrs = append(attrs, attribute.Bool(k, val)) - default: - attrs = append(attrs, attribute.String(k, fmt.Sprint(val))) - } - } - - span.AddEvent("exception", trace.WithAttributes(attrs...)) + // Record metrics for both client and server errors + if clientErr { + clientErrorsCounter.Add(ctx, 1, clientErrorCounterAttrs(exceptionType)) + } else { + serverErrorsCounter.Add(ctx, 1, serverErrorCounterAttrs(exceptionType)) } - // send to elastic APM - apm.CaptureError(ctx, cause).Send() + // Only record exception events for server errors (5xx). + // Per OTEL spec, handled errors (like 4xx responses) should not be recorded as exceptions. + // See: https://opentelemetry.io/docs/specs/semconv/general/recording-errors/ + if !clientErr { + if span := trace.SpanFromContext(ctx); span != nil && span.IsRecording() { + span.SetStatus(codes.Error, exceptionType) - // if Sentry is configured, capture the error there - if sentryClient := sentry.CurrentHub().Client(); sentryClient != nil { - if len(telemetryAttrs) > 0 { - // we have contextual information, use it to enrich the error - ctxHub := sentry.CurrentHub().Clone() - ctxHub.ConfigureScope(func(scope *sentry.Scope) { - for k, v := range telemetryAttrs { - scope.SetTag(k, fmt.Sprint(v)) + // Build attributes for the event using OTEL semantic conventions. + // See: https://opentelemetry.io/docs/specs/semconv/exceptions/exceptions-spans/ + attrs := []attribute.KeyValue{ + attribute.String("exception.type", exceptionType), + attribute.String("exception.message", cause.Error()), + attribute.String("exception.stacktrace", strings.Join(cause.Stack(), "\n")), + } + + // Add contextual information from telemetry providers. + // OpenTelemetry requires typed attributes, so we convert the values to the appropriate type. + for k, v := range telemetryAttrs { + switch val := v.(type) { + case string: + attrs = append(attrs, attribute.String(k, val)) + case int: + attrs = append(attrs, attribute.Int64(k, int64(val))) + case int64: + attrs = append(attrs, attribute.Int64(k, val)) + case uint: + attrs = append(attrs, attribute.Int64(k, int64(val))) //nolint:gosec + case uint64: + attrs = append(attrs, attribute.Int64(k, int64(val))) //nolint:gosec + case bool: + attrs = append(attrs, attribute.Bool(k, val)) + default: + attrs = append(attrs, attribute.String(k, fmt.Sprint(val))) } - }) - ctxHub.CaptureException(cause) - } else { - sentry.CaptureException(cause) + } + + span.AddEvent("exception", trace.WithAttributes(attrs...)) + } + + // send to elastic APM + apm.CaptureError(ctx, cause).Send() + + // if Sentry is configured, capture the error there + if sentryClient := sentry.CurrentHub().Client(); sentryClient != nil { + if len(telemetryAttrs) > 0 { + // we have contextual information, use it to enrich the error + ctxHub := sentry.CurrentHub().Clone() + ctxHub.ConfigureScope(func(scope *sentry.Scope) { + for k, v := range telemetryAttrs { + scope.SetTag(k, fmt.Sprint(v)) + } + }) + ctxHub.CaptureException(cause) + } else { + sentry.CaptureException(cause) + } } } @@ -382,6 +397,24 @@ func collectTelemetryContext(ctx context.Context) map[string]any { return attrs } +// isClientError checks if the error is a client error (4xx). +func isClientError(err error) bool { + // Check for explicit client error interface + var clientErr platform_http.ErrWithIsClientError + if errors.As(err, &clientErr) { + return clientErr.IsClientError() + } + + // Treat context.Canceled as a client error. In HTTP handlers, this typically + // indicates client disconnection. While it could theoretically come from + // server-side cancellation, detecting true client disconnection at the + // transport layer is complex. Go's HTTP server doesn't provide a distinct + // error type for client disconnection (see https://github.com/golang/go/issues/64465). + // The occasional misclassification is acceptable given that most context + // cancellations in request handling are client-initiated. + return errors.Is(err, context.Canceled) +} + // Retrieve retrieves an error from the registered error handler func Retrieve(ctx context.Context) ([]*StoredError, error) { eh := FromContext(ctx) diff --git a/server/contexts/ctxerr/ctxerr_test.go b/server/contexts/ctxerr/ctxerr_test.go index 97438d523a..74e9b5c607 100644 --- a/server/contexts/ctxerr/ctxerr_test.go +++ b/server/contexts/ctxerr/ctxerr_test.go @@ -387,3 +387,49 @@ func TestLogFields(t *testing.T) { require.ElementsMatch(t, c.want, got) } } + +func TestIsClientError(t *testing.T) { + tests := []struct { + name string + err error + expected bool + }{ + { + name: "nil error", + err: nil, + expected: false, + }, + { + name: "generic error", + err: errors.New("generic error"), + expected: false, + }, + { + name: "context.Canceled", + err: context.Canceled, + expected: true, + }, + { + name: "wrapped context.Canceled", + err: fmt.Errorf("wrapped: %w", context.Canceled), + expected: true, + }, + { + name: "context.DeadlineExceeded - not a client error (could be DB/upstream timeout)", + err: context.DeadlineExceeded, + expected: false, + }, + { + name: "InvalidArgumentError (implements IsClientError)", + err: &fleet.InvalidArgumentError{}, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isClientError(tt.err) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/server/contexts/ctxerr/metrics.go b/server/contexts/ctxerr/metrics.go new file mode 100644 index 0000000000..be3297bc49 --- /dev/null +++ b/server/contexts/ctxerr/metrics.go @@ -0,0 +1,56 @@ +package ctxerr + +import ( + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" +) + +var ( + meter = otel.Meter("fleet") + + // clientErrorsCounter counts client errors (4xx) by type. + // These are errors caused by client issues (bad requests, auth failures, etc.) + // and per OTEL semantic conventions should not be treated as server errors. + clientErrorsCounter metric.Int64Counter + + // serverErrorsCounter counts server errors (5xx) by type. + // These are errors caused by server issues and should be investigated. + serverErrorsCounter metric.Int64Counter +) + +func init() { + var err error + + clientErrorsCounter, err = meter.Int64Counter( + "fleet.http.client_errors", + metric.WithDescription("Count of client errors (4xx) by error type"), + metric.WithUnit("{error}"), + ) + if err != nil { + panic(err) + } + + serverErrorsCounter, err = meter.Int64Counter( + "fleet.http.server_errors", + metric.WithDescription("Count of server errors (5xx) by error type"), + metric.WithUnit("{error}"), + ) + if err != nil { + panic(err) + } +} + +// clientErrorCounterAttrs returns the metric attributes for client error counters. +func clientErrorCounterAttrs(errorType string) metric.AddOption { + return metric.WithAttributes( + attribute.String("error.type", errorType), + ) +} + +// serverErrorCounterAttrs returns the metric attributes for server error counters. +func serverErrorCounterAttrs(errorType string) metric.AddOption { + return metric.WithAttributes( + attribute.String("error.type", errorType), + ) +} diff --git a/server/datastore/mysql/mysql.go b/server/datastore/mysql/mysql.go index 6d4293a287..020757ebe2 100644 --- a/server/datastore/mysql/mysql.go +++ b/server/datastore/mysql/mysql.go @@ -37,7 +37,7 @@ import ( "github.com/go-sql-driver/mysql" "github.com/hashicorp/go-multierror" "github.com/jmoiron/sqlx" - semconv "go.opentelemetry.io/otel/semconv/v1.26.0" + semconv "go.opentelemetry.io/otel/semconv/v1.37.0" ) // Compile-time interface check @@ -348,7 +348,7 @@ var otelTracedDriverName string func init() { var err error otelTracedDriverName, err = otelsql.Register("mysql", - otelsql.WithAttributes(semconv.DBSystemMySQL), + otelsql.WithAttributes(semconv.DBSystemNameMySQL), otelsql.WithSpanOptions(otelsql.SpanOptions{ // DisableErrSkip ignores driver.ErrSkip errors which are frequently returned by the MySQL driver // when certain optional methods or paths are not implemented/taken. diff --git a/server/fleet/errors.go b/server/fleet/errors.go index 3660786ad9..ed1e6689c1 100644 --- a/server/fleet/errors.go +++ b/server/fleet/errors.go @@ -182,6 +182,11 @@ func (e PermissionError) PermissionError() []map[string]string { return forbidden } +// IsClientError implements ErrWithIsClientError. +func (e PermissionError) IsClientError() bool { + return true +} + // OTAForbiddenError is a special kind of forbidden error that intentionally // exposes information about the error so it can be shown in iPad/iPhone native // dialogs during OTA enrollment. @@ -212,6 +217,11 @@ func (e OTAForbiddenError) Internal() string { return e.InternalErr.Error() } +// IsClientError implements ErrWithIsClientError. +func (e OTAForbiddenError) IsClientError() bool { + return true +} + // licenseError is returned when the application is not properly licensed. type licenseError struct { ErrorWithUUID @@ -225,6 +235,11 @@ func (e licenseError) StatusCode() int { return http.StatusPaymentRequired } +// IsClientError implements ErrWithIsClientError. +func (e licenseError) IsClientError() bool { + return true +} + // MDMNotConfiguredError is used when an MDM endpoint or resource is accessed // without having MDM correctly configured. type MDMNotConfiguredError struct{} @@ -239,6 +254,11 @@ func (e *MDMNotConfiguredError) Error() string { return MDMNotConfiguredMessage } +// IsClientError implements ErrWithIsClientError. +func (e *MDMNotConfiguredError) IsClientError() bool { + return true +} + // WindowsMDMNotConfiguredError is used when an MDM endpoint or resource is accessed // without having Windows MDM correctly configured. type WindowsMDMNotConfiguredError struct{} @@ -253,6 +273,11 @@ func (e *WindowsMDMNotConfiguredError) Error() string { return WindowsMDMNotConfiguredMessage } +// IsClientError implements ErrWithIsClientError. +func (e *WindowsMDMNotConfiguredError) IsClientError() bool { + return true +} + // AndroidMDMNotConfiguredError is used when an MDM endpoint or resource is accessed // without having Android MDM correctly configured. type AndroidMDMNotConfiguredError struct{} @@ -267,6 +292,11 @@ func (e *AndroidMDMNotConfiguredError) Error() string { return AndroidMDMNotConfiguredMessage } +// IsClientError implements ErrWithIsClientError. +func (e *AndroidMDMNotConfiguredError) IsClientError() bool { + return true +} + // NotConfiguredError is a generic "not configured" error that can be used // when expected configuration is missing. type NotConfiguredError struct{} @@ -433,6 +463,13 @@ func (e OrbitError) StatusCode() int { return e.code } +// IsClientError implements ErrWithIsClientError. +// Returns true for 4xx status codes, false for 5xx. +func (e OrbitError) IsClientError() bool { + code := e.StatusCode() + return code >= 400 && code < 500 +} + func NewOrbitIDPAuthRequiredError() *OrbitError { return &OrbitError{ Message: "END_USER_AUTH_REQUIRED", @@ -510,6 +547,11 @@ func (e ConflictError) IsConflict() bool { return true } +// IsClientError implements ErrWithIsClientError. +func (e ConflictError) IsClientError() bool { + return true +} + // Errorer is an alias for platform_http.Errorer. type Errorer = platform_http.Errorer diff --git a/server/platform/http/errors.go b/server/platform/http/errors.go index 5984c395a9..0b6ae8889a 100644 --- a/server/platform/http/errors.go +++ b/server/platform/http/errors.go @@ -81,6 +81,11 @@ func (e BadRequestError) Internal() string { return "" } +// IsClientError implements ErrWithIsClientError. +func (e BadRequestError) IsClientError() bool { + return true +} + // UserMessageError is an error that wraps another error with a user-friendly message. type UserMessageError struct { error @@ -111,6 +116,13 @@ func (e UserMessageError) StatusCode() int { return http.StatusUnprocessableEntity } +// IsClientError implements ErrWithIsClientError. +// Returns true for 4xx status codes, false for 5xx. +func (e UserMessageError) IsClientError() bool { + code := e.StatusCode() + return code >= 400 && code < 500 +} + var rxJSONUnknownField = regexp.MustCompile(`^json: unknown field "(.+)"$`) // IsJSONUnknownFieldError returns true if err is a JSON unknown field error. @@ -266,6 +278,11 @@ func (e AuthFailedError) StatusCode() int { return http.StatusUnauthorized } +// IsClientError implements ErrWithIsClientError. +func (e AuthFailedError) IsClientError() bool { + return true +} + // AuthRequiredError is returned when authentication is required. type AuthRequiredError struct { // internal is the reason that should only be logged internally @@ -294,6 +311,11 @@ func (e AuthRequiredError) StatusCode() int { return http.StatusUnauthorized } +// IsClientError implements ErrWithIsClientError. +func (e AuthRequiredError) IsClientError() bool { + return true +} + // AuthHeaderRequiredError is returned when an authorization header is required. type AuthHeaderRequiredError struct { // internal is the reason that should only be logged internally @@ -324,6 +346,11 @@ func (e AuthHeaderRequiredError) StatusCode() int { return http.StatusUnauthorized } +// IsClientError implements ErrWithIsClientError. +func (e AuthHeaderRequiredError) IsClientError() bool { + return true +} + // ErrPasswordResetRequired is returned when a password reset is required. var ErrPasswordResetRequired = &passwordResetRequiredError{} @@ -341,6 +368,11 @@ func (e passwordResetRequiredError) StatusCode() int { return http.StatusUnauthorized } +// IsClientError implements ErrWithIsClientError. +func (e passwordResetRequiredError) IsClientError() bool { + return true +} + // ForbiddenErrorMessage is the error message that should be returned to // clients when an action is forbidden. It is intentionally vague to prevent // disclosing information that a client should not have access to. diff --git a/server/platform/mysql/errors.go b/server/platform/mysql/errors.go index 33b10be646..e75e00b852 100644 --- a/server/platform/mysql/errors.go +++ b/server/platform/mysql/errors.go @@ -55,6 +55,11 @@ func (e *NotFoundError) IsNotFound() bool { return true } +// IsClientError implements ErrWithIsClientError. +func (e *NotFoundError) IsClientError() bool { + return true +} + // Is helps so that errors.Is(err, sql.ErrNoRows) returns true for an // error of type *NotFoundError, without having to wrap sql.ErrNoRows // explicitly. diff --git a/server/service/integration_logger_test.go b/server/service/integration_logger_test.go index cbd0be32fb..ed34edd8be 100644 --- a/server/service/integration_logger_test.go +++ b/server/service/integration_logger_test.go @@ -133,7 +133,7 @@ func (s *integrationLoggerTestSuite) TestLoggerLogin() { expectedStatus: http.StatusUnauthorized, expectedLogs: []logEntry{ {"email", testUsers["admin1"].Email}, - {"level", "error"}, + {"level", "info"}, {"internal", "invalid password"}, }, }, @@ -142,7 +142,7 @@ func (s *integrationLoggerTestSuite) TestLoggerLogin() { expectedStatus: http.StatusUnauthorized, expectedLogs: []logEntry{ {"email", "h4x0r@3x4mp13.c0m"}, - {"level", "error"}, + {"level", "info"}, {"internal", "user not found"}, }, }, @@ -310,7 +310,7 @@ func (s *integrationLoggerTestSuite) TestEnrollOsqueryLogsErrors() { require.Len(t, parts, 1) logData := make(map[string]json.RawMessage) require.NoError(t, json.Unmarshal([]byte(parts[0]), &logData)) - assert.Equal(t, `"error"`, string(logData["level"])) + assert.Equal(t, `"info"`, string(logData["level"])) assert.Contains(t, string(logData["err"]), `"enroll failed:`) assert.Contains(t, string(logData["err"]), `no matching secret found`) } diff --git a/server/service/osquery.go b/server/service/osquery.go index 9cb4505ed5..2057ae3541 100644 --- a/server/service/osquery.go +++ b/server/service/osquery.go @@ -99,7 +99,7 @@ func (svc *Service) EnrollOsquery(ctx context.Context, enrollSecret, hostIdentif // skipauth: Authorization is currently for user endpoints only. svc.authz.SkipAuthorization(ctx) - logging.WithExtras(ctx, "hostIdentifier", hostIdentifier) + logging.WithLevel(logging.WithExtras(ctx, "hostIdentifier", hostIdentifier), level.Info) secret, err := svc.ds.VerifyEnrollSecret(ctx, enrollSecret) if err != nil { diff --git a/server/service/transport_error.go b/server/service/transport_error.go index c3e617808c..ce347e0f66 100644 --- a/server/service/transport_error.go +++ b/server/service/transport_error.go @@ -90,6 +90,19 @@ func (e *OsqueryError) Status() int { return e.StatusCode } +// IsClientError implements ErrWithIsClientError. +// OsqueryError is a client error when the node is invalid (auth failure) +// or when the status code is in the 4xx range. +func (e *OsqueryError) IsClientError() bool { + if e.nodeInvalid { + return true + } + if e.StatusCode >= 400 && e.StatusCode < 500 { + return true + } + return false +} + func NewOsqueryError(message string, nodeInvalid bool) *OsqueryError { return &OsqueryError{ message: message,