Added missing OpenTelemetry instrumentation to several API endpoints. (#32960)
Fixes #32331 Manually tested all paths. `/test` path removed in https://github.com/fleetdm/fleet/pull/32962 Also added support for sending errors to OpenTelemetry, like we do for APM/Sentry. # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added OpenTelemetry tracing across core HTTP endpoints (health, version, assets, metrics, enroll/root, debug, Apple MDM, SCEP, SCIM) with dynamic per-request route instrumentation. * Enhanced error reporting to include OpenTelemetry spans/events with contextual user/host attributes. * **Tests** * Added unit tests validating SCIM and error-handling telemetry, span naming, and sensitive-data redaction. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1 @@
|
||||
* Added missing OpenTelemetry instrumentation to several API endpoints.
|
||||
+13
-11
@@ -61,6 +61,7 @@ import (
|
||||
"github.com/fleetdm/fleet/v4/server/service/async"
|
||||
"github.com/fleetdm/fleet/v4/server/service/conditional_access_microsoft_proxy"
|
||||
"github.com/fleetdm/fleet/v4/server/service/middleware/endpoint_utils"
|
||||
otelmw "github.com/fleetdm/fleet/v4/server/service/middleware/otel"
|
||||
"github.com/fleetdm/fleet/v4/server/service/redis_key_value"
|
||||
"github.com/fleetdm/fleet/v4/server/service/redis_lock"
|
||||
"github.com/fleetdm/fleet/v4/server/service/redis_policy_set"
|
||||
@@ -1234,9 +1235,9 @@ the way that the Fleet server works.
|
||||
launcher := launcher.New(svc, logger, grpc.NewServer(), healthCheckers)
|
||||
|
||||
rootMux := http.NewServeMux()
|
||||
rootMux.Handle("/healthz", service.PrometheusMetricsHandler("healthz", health.Handler(httpLogger, healthCheckers)))
|
||||
rootMux.Handle("/version", service.PrometheusMetricsHandler("version", version.Handler()))
|
||||
rootMux.Handle("/assets/", service.PrometheusMetricsHandler("static_assets", service.ServeStaticAssets("/assets/")))
|
||||
rootMux.Handle("/healthz", service.PrometheusMetricsHandler("healthz", otelmw.WrapHandler(health.Handler(httpLogger, healthCheckers), "/healthz", config)))
|
||||
rootMux.Handle("/version", service.PrometheusMetricsHandler("version", otelmw.WrapHandler(version.Handler(), "/version", config)))
|
||||
rootMux.Handle("/assets/", service.PrometheusMetricsHandler("static_assets", otelmw.WrapHandlerDynamic(service.ServeStaticAssets("/assets/"), config)))
|
||||
|
||||
if len(config.Server.PrivateKey) > 0 {
|
||||
commander := apple_mdm.NewMDMAppleCommander(mdmStorage, mdmPushService)
|
||||
@@ -1281,6 +1282,7 @@ the way that the Fleet server works.
|
||||
ddmService,
|
||||
commander,
|
||||
appCfg.ServerSettings.ServerURL,
|
||||
config,
|
||||
); err != nil {
|
||||
initFatal(err, "setup mdm apple services")
|
||||
}
|
||||
@@ -1288,10 +1290,10 @@ the way that the Fleet server works.
|
||||
|
||||
if license.IsPremium() {
|
||||
// SCEP proxy (for NDES, etc.)
|
||||
if err = service.RegisterSCEPProxy(rootMux, ds, logger, nil); err != nil {
|
||||
if err = service.RegisterSCEPProxy(rootMux, ds, logger, nil, &config); err != nil {
|
||||
initFatal(err, "setup SCEP proxy")
|
||||
}
|
||||
if err = scim.RegisterSCIM(rootMux, ds, svc, logger); err != nil {
|
||||
if err = scim.RegisterSCIM(rootMux, ds, svc, logger, &config); err != nil {
|
||||
initFatal(err, "setup SCIM")
|
||||
}
|
||||
// Host identify SCEP feature only works if a private key has been set up
|
||||
@@ -1300,7 +1302,7 @@ the way that the Fleet server works.
|
||||
if err != nil {
|
||||
initFatal(err, "setup host identity SCEP depot")
|
||||
}
|
||||
if err = hostidentity.RegisterSCEP(rootMux, hostIdentitySCEPDepot, ds, logger); err != nil {
|
||||
if err = hostidentity.RegisterSCEP(rootMux, hostIdentitySCEPDepot, ds, logger, &config); err != nil {
|
||||
initFatal(err, "setup host identity SCEP")
|
||||
}
|
||||
} else {
|
||||
@@ -1312,12 +1314,12 @@ the way that the Fleet server works.
|
||||
rootMux.Handle("/metrics", basicAuthHandler(
|
||||
config.Prometheus.BasicAuth.Username,
|
||||
config.Prometheus.BasicAuth.Password,
|
||||
service.PrometheusMetricsHandler("metrics", promhttp.Handler()),
|
||||
service.PrometheusMetricsHandler("metrics", otelmw.WrapHandler(promhttp.Handler(), "/metrics", config)),
|
||||
))
|
||||
} else {
|
||||
if config.Prometheus.BasicAuth.Disable {
|
||||
level.Info(logger).Log("msg", "metrics endpoint enabled with http basic auth disabled")
|
||||
rootMux.Handle("/metrics", service.PrometheusMetricsHandler("metrics", promhttp.Handler()))
|
||||
rootMux.Handle("/metrics", service.PrometheusMetricsHandler("metrics", otelmw.WrapHandler(promhttp.Handler(), "/metrics", config)))
|
||||
} else {
|
||||
level.Info(logger).Log("msg", "metrics endpoint disabled (http basic auth credentials not set)")
|
||||
}
|
||||
@@ -1435,13 +1437,13 @@ the way that the Fleet server works.
|
||||
rootMux.Handle("/api/v1/fleet/scim/details", apiHandler)
|
||||
rootMux.Handle("/api/latest/fleet/scim/details", apiHandler)
|
||||
|
||||
rootMux.Handle("/enroll", endUserEnrollOTAHandler)
|
||||
rootMux.Handle("/", frontendHandler)
|
||||
rootMux.Handle("/enroll", otelmw.WrapHandler(endUserEnrollOTAHandler, "/enroll", config))
|
||||
rootMux.Handle("/", otelmw.WrapHandler(frontendHandler, "/", config))
|
||||
|
||||
debugHandler := &debugMux{
|
||||
fleetAuthenticatedHandler: service.MakeDebugHandler(svc, config, logger, eh, ds),
|
||||
}
|
||||
rootMux.Handle("/debug/", debugHandler)
|
||||
rootMux.Handle("/debug/", otelmw.WrapHandlerDynamic(debugHandler, config))
|
||||
|
||||
if debug {
|
||||
// Add debug endpoints with a random
|
||||
|
||||
+93
-7
@@ -3,21 +3,24 @@ package scim
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/elimity-com/scim"
|
||||
"github.com/elimity-com/scim/errors"
|
||||
scimerrors "github.com/elimity-com/scim/errors"
|
||||
"github.com/elimity-com/scim/optional"
|
||||
"github.com/elimity-com/scim/schema"
|
||||
"github.com/fleetdm/fleet/v4/server/authz"
|
||||
"github.com/fleetdm/fleet/v4/server/config"
|
||||
"github.com/fleetdm/fleet/v4/server/fleet"
|
||||
"github.com/fleetdm/fleet/v4/server/service/middleware/auth"
|
||||
"github.com/fleetdm/fleet/v4/server/service/middleware/log"
|
||||
kitlog "github.com/go-kit/log"
|
||||
"github.com/go-kit/log/level"
|
||||
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -29,7 +32,11 @@ func RegisterSCIM(
|
||||
ds fleet.Datastore,
|
||||
svc fleet.Service,
|
||||
logger kitlog.Logger,
|
||||
fleetConfig *config.FleetConfig,
|
||||
) error {
|
||||
if fleetConfig == nil {
|
||||
return errors.New("fleet config is nil")
|
||||
}
|
||||
config := scim.ServiceProviderConfig{
|
||||
DocumentationURI: optional.NewString("https://fleetdm.com/docs/get-started/why-fleet"),
|
||||
MaxResults: maxResults,
|
||||
@@ -209,7 +216,7 @@ func RegisterSCIM(
|
||||
return err
|
||||
}
|
||||
|
||||
// TODO: Add APM/OpenTelemetry tracing and Prometheus middleware
|
||||
// Apply middleware including OTEL instrumentation
|
||||
applyMiddleware := func(prefix string, server http.Handler) http.Handler {
|
||||
handler := http.StripPrefix(prefix, server)
|
||||
handler = AuthorizationMiddleware(authorizer, scimLogger, handler)
|
||||
@@ -222,11 +229,90 @@ func RegisterSCIM(
|
||||
|
||||
// We cannot use Go URL path pattern like {version} because the http.StripPrefix method
|
||||
// that gets us to the root SCIM path does not support wildcards: https://github.com/golang/go/issues/64909
|
||||
mux.Handle("/api/v1/fleet/scim/", applyMiddleware("/api/v1/fleet/scim", server))
|
||||
mux.Handle("/api/latest/fleet/scim/", applyMiddleware("/api/latest/fleet/scim", server))
|
||||
// Apply OTEL instrumentation at the mux level (outermost)
|
||||
mux.Handle("/api/v1/fleet/scim/", scimOTELMiddleware(applyMiddleware("/api/v1/fleet/scim", server), "/api/v1/fleet/scim", *fleetConfig))
|
||||
mux.Handle("/api/latest/fleet/scim/", scimOTELMiddleware(applyMiddleware("/api/latest/fleet/scim", server), "/api/latest/fleet/scim", *fleetConfig))
|
||||
return nil
|
||||
}
|
||||
|
||||
// scimOTELMiddleware provides OpenTelemetry instrumentation for SCIM endpoints
|
||||
// It creates proper span names without exposing sensitive IDs
|
||||
func scimOTELMiddleware(next http.Handler, prefix string, cfg config.FleetConfig) http.Handler {
|
||||
if !cfg.Logging.TracingEnabled || cfg.Logging.TracingType != "opentelemetry" {
|
||||
return next
|
||||
}
|
||||
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
// Determine the SCIM route pattern based on the path
|
||||
// OTEL is the outermost middleware, so we see the full path including prefix
|
||||
fullPath := r.URL.Path
|
||||
|
||||
// Remove the prefix to get the SCIM-specific path
|
||||
scimPath := strings.TrimPrefix(fullPath, prefix)
|
||||
// Handle both "/Schemas" and "Schemas" by trimming the leading slash
|
||||
scimPath = strings.TrimPrefix(scimPath, "/")
|
||||
|
||||
var route string
|
||||
|
||||
// Debug: Log the actual path we're processing
|
||||
// fmt.Printf("DEBUG: SCIM OTEL fullPath=%q, scimPath=%q\n", fullPath, scimPath)
|
||||
|
||||
// Normalize the path to create a route pattern without exposing IDs
|
||||
switch {
|
||||
case strings.HasPrefix(scimPath, "Users"):
|
||||
segments := strings.Split(scimPath, "/")
|
||||
if len(segments) == 1 || (len(segments) == 2 && segments[1] == "") {
|
||||
route = prefix + "/Users"
|
||||
} else {
|
||||
// Individual user operations - don't expose the user ID
|
||||
route = prefix + "/Users/{id}"
|
||||
}
|
||||
case strings.HasPrefix(scimPath, "Groups"):
|
||||
segments := strings.Split(scimPath, "/")
|
||||
if len(segments) == 1 || (len(segments) == 2 && segments[1] == "") {
|
||||
route = prefix + "/Groups"
|
||||
} else {
|
||||
// Individual group operations - don't expose the group ID
|
||||
route = prefix + "/Groups/{id}"
|
||||
}
|
||||
case strings.HasPrefix(scimPath, "Schemas"):
|
||||
segments := strings.Split(scimPath, "/")
|
||||
if len(segments) == 1 || (len(segments) == 2 && segments[1] == "") {
|
||||
route = prefix + "/Schemas"
|
||||
} else {
|
||||
route = prefix + "/Schemas/{id}"
|
||||
}
|
||||
case scimPath == "ServiceProviderConfig" || scimPath == "ServiceProviderConfig/":
|
||||
route = prefix + "/ServiceProviderConfig"
|
||||
case scimPath == "ResourceTypes" || scimPath == "ResourceTypes/":
|
||||
route = prefix + "/ResourceTypes"
|
||||
default:
|
||||
// For any other path, use the full path but check for potential IDs
|
||||
// If the path looks like it might contain an ID (has multiple segments),
|
||||
// we should sanitize it
|
||||
segments := strings.Split(strings.Trim(scimPath, "/"), "/")
|
||||
if len(segments) > 1 {
|
||||
// Might be something like CustomResource/123
|
||||
// Replace the last segment with {id} if it looks like an ID
|
||||
route = prefix + "/" + segments[0] + "/{id}"
|
||||
} else {
|
||||
// Single segment path, use as is
|
||||
route = prefix + "/" + scimPath
|
||||
}
|
||||
}
|
||||
|
||||
// Create the instrumented handler with the proper route
|
||||
instrumentedHandler := otelhttp.NewHandler(
|
||||
otelhttp.WithRouteTag(route, next),
|
||||
"", // Empty operation name - will be set by span name formatter
|
||||
otelhttp.WithSpanNameFormatter(func(operation string, req *http.Request) string {
|
||||
return req.Method + " " + route
|
||||
}),
|
||||
)
|
||||
instrumentedHandler.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
|
||||
// LastRequestMiddleware saves the details of the last request to SCIM endpoints in the datastore.
|
||||
// These details can be used as a debug tool by the Fleet admin to see if SCIM integration is working.
|
||||
func LastRequestMiddleware(ds fleet.Datastore, logger kitlog.Logger, next http.Handler) http.Handler {
|
||||
@@ -253,13 +339,13 @@ func LastRequestMiddleware(ds fleet.Datastore, logger kitlog.Logger, next http.H
|
||||
case multi.statusCode >= 400:
|
||||
status = "error"
|
||||
// Attempt to parse the response body as a SCIM error.
|
||||
var parsedScimError errors.ScimError
|
||||
var parsedScimError scimerrors.ScimError
|
||||
if err := json.Unmarshal(multi.body.Bytes(), &parsedScimError); err == nil {
|
||||
details = parsedScimError.Detail
|
||||
} else {
|
||||
details = multi.body.String()
|
||||
}
|
||||
if multi.statusCode == errors.ScimErrorInvalidValue.Status && details == errors.ScimErrorInvalidValue.Detail &&
|
||||
if multi.statusCode == scimerrors.ScimErrorInvalidValue.Status && details == scimerrors.ScimErrorInvalidValue.Detail &&
|
||||
strings.Contains(r.URL.Path, "/Users") {
|
||||
// We customize the error message here since we can't do it inside the 3rd party SCIM library.
|
||||
details = `Missing required attributes. "userName", "givenName", and "familyName" are required. Please configure your identity provider to send required attributes to Fleet.`
|
||||
@@ -294,7 +380,7 @@ func AuthorizationMiddleware(authorizer *authz.Authorizer, logger kitlog.Logger,
|
||||
}
|
||||
|
||||
func errorHandler(w http.ResponseWriter, logger kitlog.Logger, detail string, status int) {
|
||||
scimErr := errors.ScimError{
|
||||
scimErr := scimerrors.ScimError{
|
||||
Status: status,
|
||||
Detail: detail,
|
||||
}
|
||||
|
||||
@@ -0,0 +1,211 @@
|
||||
package scim
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/server/config"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.opentelemetry.io/otel/sdk/trace"
|
||||
"go.opentelemetry.io/otel/sdk/trace/tracetest"
|
||||
)
|
||||
|
||||
func TestSCIMOTELMiddleware(t *testing.T) {
|
||||
t.Parallel()
|
||||
testCases := []struct {
|
||||
name string
|
||||
path string
|
||||
method string
|
||||
expectedSpan string
|
||||
}{
|
||||
{
|
||||
name: "Users list",
|
||||
path: "Users",
|
||||
method: "GET",
|
||||
expectedSpan: "GET /api/v1/fleet/scim/Users",
|
||||
},
|
||||
{
|
||||
name: "Users list with trailing slash",
|
||||
path: "Users/",
|
||||
method: "GET",
|
||||
expectedSpan: "GET /api/v1/fleet/scim/Users",
|
||||
},
|
||||
{
|
||||
name: "Individual user - hides ID",
|
||||
path: "Users/12345",
|
||||
method: "GET",
|
||||
expectedSpan: "GET /api/v1/fleet/scim/Users/{id}",
|
||||
},
|
||||
{
|
||||
name: "Update user - hides ID",
|
||||
path: "Users/67890",
|
||||
method: "PATCH",
|
||||
expectedSpan: "PATCH /api/v1/fleet/scim/Users/{id}",
|
||||
},
|
||||
{
|
||||
name: "Groups list",
|
||||
path: "Groups",
|
||||
method: "GET",
|
||||
expectedSpan: "GET /api/v1/fleet/scim/Groups",
|
||||
},
|
||||
{
|
||||
name: "Individual group - hides ID",
|
||||
path: "Groups/abc-def-123",
|
||||
method: "PUT",
|
||||
expectedSpan: "PUT /api/v1/fleet/scim/Groups/{id}",
|
||||
},
|
||||
{
|
||||
name: "Schemas",
|
||||
path: "Schemas",
|
||||
method: "GET",
|
||||
expectedSpan: "GET /api/v1/fleet/scim/Schemas",
|
||||
},
|
||||
{
|
||||
name: "Individual schema",
|
||||
path: "Schemas/urn:ietf:params:scim:schemas:core:2.0:User",
|
||||
method: "GET",
|
||||
expectedSpan: "GET /api/v1/fleet/scim/Schemas/{id}",
|
||||
},
|
||||
{
|
||||
name: "Service provider config",
|
||||
path: "ServiceProviderConfig",
|
||||
method: "GET",
|
||||
expectedSpan: "GET /api/v1/fleet/scim/ServiceProviderConfig",
|
||||
},
|
||||
{
|
||||
name: "Resource types",
|
||||
path: "ResourceTypes",
|
||||
method: "GET",
|
||||
expectedSpan: "GET /api/v1/fleet/scim/ResourceTypes",
|
||||
},
|
||||
{
|
||||
name: "Unknown path - uses full path",
|
||||
path: "SomethingElse",
|
||||
method: "GET",
|
||||
expectedSpan: "GET /api/v1/fleet/scim/SomethingElse",
|
||||
},
|
||||
{
|
||||
name: "Bulk operations endpoint",
|
||||
path: "Bulk",
|
||||
method: "POST",
|
||||
expectedSpan: "POST /api/v1/fleet/scim/Bulk",
|
||||
},
|
||||
{
|
||||
name: "Search endpoint",
|
||||
path: ".search",
|
||||
method: "POST",
|
||||
expectedSpan: "POST /api/v1/fleet/scim/.search",
|
||||
},
|
||||
{
|
||||
name: "Unknown resource with ID - hides ID",
|
||||
path: "CustomResource/abc123",
|
||||
method: "GET",
|
||||
expectedSpan: "GET /api/v1/fleet/scim/CustomResource/{id}",
|
||||
},
|
||||
{
|
||||
name: "Unknown nested path with ID - hides ID",
|
||||
path: "Custom/Resource/123",
|
||||
method: "DELETE",
|
||||
expectedSpan: "DELETE /api/v1/fleet/scim/Custom/{id}",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
// Create a test span recorder
|
||||
sr := tracetest.NewSpanRecorder()
|
||||
tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr))
|
||||
|
||||
// Create test configuration with OTEL enabled
|
||||
cfg := config.FleetConfig{
|
||||
Logging: config.LoggingConfig{
|
||||
TracingEnabled: true,
|
||||
TracingType: "opentelemetry",
|
||||
},
|
||||
}
|
||||
|
||||
// Create a test handler that just returns 200 OK
|
||||
testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})
|
||||
|
||||
// Wrap with SCIM OTEL middleware
|
||||
tracer := tp.Tracer("test")
|
||||
wrappedHandler := scimOTELMiddleware(testHandler, "/api/v1/fleet/scim", cfg)
|
||||
|
||||
// Create request - now OTEL runs before StripPrefix so it sees the full path
|
||||
req := httptest.NewRequest(tc.method, "/api/v1/fleet/scim/"+tc.path, nil)
|
||||
|
||||
// Add span to context
|
||||
ctx, span := tracer.Start(req.Context(), "test-parent-span")
|
||||
defer span.End()
|
||||
req = req.WithContext(ctx)
|
||||
|
||||
// Execute request
|
||||
w := httptest.NewRecorder()
|
||||
wrappedHandler.ServeHTTP(w, req)
|
||||
|
||||
// Force span to end
|
||||
span.End()
|
||||
|
||||
// Check spans
|
||||
spans := sr.Ended()
|
||||
require.GreaterOrEqual(t, len(spans), 2, "Should have at least parent and child spans")
|
||||
|
||||
// Find the SCIM span (should be the second one, after the parent)
|
||||
var scimSpan trace.ReadOnlySpan
|
||||
for _, s := range spans {
|
||||
if s.Name() == tc.expectedSpan {
|
||||
scimSpan = s
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
require.NotNil(t, scimSpan, "Should find SCIM span with name: %s", tc.expectedSpan)
|
||||
assert.Equal(t, tc.expectedSpan, scimSpan.Name())
|
||||
|
||||
// Check that the route tag is set correctly (without exposing IDs)
|
||||
attrs := scimSpan.Attributes()
|
||||
for _, attr := range attrs {
|
||||
if string(attr.Key) == "http.route" {
|
||||
// The route should match the pattern, not the actual path
|
||||
assert.NotContains(t, attr.Value.AsString(), "123", "Should not expose user ID")
|
||||
assert.NotContains(t, attr.Value.AsString(), "67890", "Should not expose user ID")
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSCIMOTELMiddleware_Disabled(t *testing.T) {
|
||||
t.Parallel()
|
||||
// Create test configuration with OTEL disabled
|
||||
cfg := config.FleetConfig{
|
||||
Logging: config.LoggingConfig{
|
||||
TracingEnabled: false,
|
||||
},
|
||||
}
|
||||
|
||||
// Create a test handler
|
||||
called := false
|
||||
testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
called = true
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})
|
||||
|
||||
// Wrap with SCIM OTEL middleware
|
||||
wrappedHandler := scimOTELMiddleware(testHandler, "/api/v1/fleet/scim", cfg)
|
||||
|
||||
// Create request - OTEL sees the full path now
|
||||
req := httptest.NewRequest("GET", "/api/v1/fleet/scim/Users", nil)
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
// Execute request
|
||||
wrappedHandler.ServeHTTP(w, req)
|
||||
|
||||
// Should have called the handler without any OTEL instrumentation
|
||||
assert.True(t, called, "Handler should have been called")
|
||||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
}
|
||||
@@ -19,11 +19,13 @@ import (
|
||||
|
||||
"github.com/cenkalti/backoff/v4"
|
||||
"github.com/fleetdm/fleet/v4/ee/server/service/hostidentity/types"
|
||||
"github.com/fleetdm/fleet/v4/server/config"
|
||||
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
|
||||
"github.com/fleetdm/fleet/v4/server/fleet"
|
||||
"github.com/fleetdm/fleet/v4/server/mdm/assets"
|
||||
scepdepot "github.com/fleetdm/fleet/v4/server/mdm/scep/depot"
|
||||
scepserver "github.com/fleetdm/fleet/v4/server/mdm/scep/server"
|
||||
"github.com/fleetdm/fleet/v4/server/service/middleware/otel"
|
||||
"github.com/go-kit/kit/log"
|
||||
kitlog "github.com/go-kit/log"
|
||||
"github.com/go-kit/log/level"
|
||||
@@ -66,7 +68,11 @@ func RegisterSCEP(
|
||||
scepStorage scepdepot.Depot,
|
||||
ds fleet.Datastore,
|
||||
logger kitlog.Logger,
|
||||
fleetConfig *config.FleetConfig,
|
||||
) error {
|
||||
if fleetConfig == nil {
|
||||
return errors.New("fleet config is nil")
|
||||
}
|
||||
err := initAssets(ds)
|
||||
if err != nil {
|
||||
return fmt.Errorf("initializing host identity assets: %w", err)
|
||||
@@ -89,12 +95,12 @@ func RegisterSCEP(
|
||||
e.GetEndpoint = scepserver.EndpointLoggingMiddleware(scepLogger)(e.GetEndpoint)
|
||||
e.PostEndpoint = scepserver.EndpointLoggingMiddleware(scepLogger)(e.PostEndpoint)
|
||||
|
||||
// Note: Monitoring (APM/OpenTel) is missing for this SCEP server.
|
||||
// In addition, the scepserver error handler does not send errors to APM/Sentry/Redis.
|
||||
// The scepserver error handler does not send errors to APM/Sentry/Redis.
|
||||
// It should be enhanced to do so if/when we start monitoring error traces.
|
||||
// This note also applies to the other SCEP servers we use.
|
||||
// That is why we're not using ctxerr wrappers here.
|
||||
scepHandler := scepserver.MakeHTTPHandler(e, scepService, scepLogger)
|
||||
scepHandler = otel.WrapHandler(scepHandler, scepPath, *fleetConfig)
|
||||
mux.Handle(scepPath, scepHandler)
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -25,6 +25,9 @@ import (
|
||||
"github.com/fleetdm/fleet/v4/server/fleet"
|
||||
"github.com/getsentry/sentry-go"
|
||||
"go.elastic.co/apm/v2"
|
||||
"go.opentelemetry.io/otel/attribute"
|
||||
"go.opentelemetry.io/otel/codes"
|
||||
"go.opentelemetry.io/otel/trace"
|
||||
)
|
||||
|
||||
type key int
|
||||
@@ -291,20 +294,61 @@ func FromContext(ctx context.Context) Handler {
|
||||
|
||||
// Handle handles err by passing it to the registered error handler,
|
||||
// deduplicating it and storing it for a configured duration. It also takes
|
||||
// care of sending it to the configured APM, if any.
|
||||
// care of sending it to the configured OpenTelemetry/APM/Sentry, if any.
|
||||
func Handle(ctx context.Context, err error) {
|
||||
if err == nil {
|
||||
return
|
||||
}
|
||||
|
||||
// as a last resource, wrap the error if there isn't
|
||||
// a FleetError in the chain
|
||||
var ferr *FleetError
|
||||
if !errors.As(err, &ferr) {
|
||||
err = Wrap(ctx, err, "missing FleetError in chain")
|
||||
wrapped := wrapError(ctx, "missing FleetError in chain", err, nil)
|
||||
if wrapped == nil {
|
||||
return // shouldn't happen since err is not nil, but be safe
|
||||
}
|
||||
// wrapError returns an error interface, but we know it's a *FleetError
|
||||
ferr = wrapped.(*FleetError)
|
||||
}
|
||||
|
||||
cause := err
|
||||
if ferr := FleetCause(err); ferr != nil {
|
||||
cause := ferr
|
||||
if rootCause := FleetCause(ferr); rootCause != nil {
|
||||
// use the FleetCause error so we send the most relevant stacktrace to APM
|
||||
// (the one from the initial New/Wrap call).
|
||||
cause = ferr
|
||||
cause = rootCause
|
||||
}
|
||||
|
||||
// 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.
|
||||
span.SetStatus(codes.Error, cause.Error())
|
||||
|
||||
// Build attributes for the exception event
|
||||
attrs := []attribute.KeyValue{
|
||||
attribute.String("exception.type", fmt.Sprintf("%T", cause)),
|
||||
attribute.String("exception.message", cause.Error()),
|
||||
attribute.String("exception.stacktrace", strings.Join(cause.Stack(), "\n")),
|
||||
}
|
||||
|
||||
// Add contextual information if available (same as Sentry)
|
||||
v, _ := viewer.FromContext(ctx)
|
||||
h, _ := host.FromContext(ctx)
|
||||
|
||||
if v.User != nil {
|
||||
attrs = append(attrs,
|
||||
// Not sending the email here as it may contain sensitive information (PII).
|
||||
attribute.Int64("user.id", int64(v.User.ID)), //nolint:gosec
|
||||
)
|
||||
} else if h != nil {
|
||||
attrs = append(attrs,
|
||||
attribute.String("host.hostname", h.Hostname),
|
||||
attribute.Int64("host.id", int64(h.ID)), //nolint:gosec
|
||||
)
|
||||
}
|
||||
|
||||
span.AddEvent("exception", trace.WithAttributes(attrs...))
|
||||
}
|
||||
|
||||
// send to elastic APM
|
||||
@@ -338,7 +382,7 @@ func Handle(ctx context.Context, err error) {
|
||||
}
|
||||
|
||||
if eh := FromContext(ctx); eh != nil {
|
||||
eh.Store(err)
|
||||
eh.Store(ferr)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,129 @@
|
||||
package ctxerr
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/server/contexts/host"
|
||||
"github.com/fleetdm/fleet/v4/server/contexts/viewer"
|
||||
"github.com/fleetdm/fleet/v4/server/fleet"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.opentelemetry.io/otel/sdk/trace"
|
||||
"go.opentelemetry.io/otel/sdk/trace/tracetest"
|
||||
)
|
||||
|
||||
func TestHandleSendsContextToOTEL(t *testing.T) {
|
||||
t.Parallel()
|
||||
testCases := []struct {
|
||||
name string
|
||||
setupContext func(context.Context) context.Context
|
||||
errorMessage string
|
||||
expectedAttrs map[string]any // expected attributes in the exception event
|
||||
}{
|
||||
{
|
||||
name: "with user context",
|
||||
setupContext: func(ctx context.Context) context.Context {
|
||||
testUser := &fleet.User{
|
||||
ID: 123,
|
||||
Email: "test@example.com",
|
||||
}
|
||||
return viewer.NewContext(ctx, viewer.Viewer{User: testUser})
|
||||
},
|
||||
errorMessage: "test error with user context",
|
||||
expectedAttrs: map[string]any{
|
||||
"user.id": int64(123),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "with host context",
|
||||
setupContext: func(ctx context.Context) context.Context {
|
||||
testHost := &fleet.Host{
|
||||
ID: 456,
|
||||
Hostname: "test-host.example.com",
|
||||
}
|
||||
return host.NewContext(ctx, testHost)
|
||||
},
|
||||
errorMessage: "test error with host context",
|
||||
expectedAttrs: map[string]any{
|
||||
"host.hostname": "test-host.example.com",
|
||||
"host.id": int64(456),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "without additional context",
|
||||
setupContext: func(ctx context.Context) context.Context {
|
||||
return ctx // no additional context
|
||||
},
|
||||
errorMessage: "test error without context",
|
||||
expectedAttrs: map[string]any{},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
// Create a test span recorder and tracer provider
|
||||
sr := tracetest.NewSpanRecorder()
|
||||
tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr))
|
||||
|
||||
// Create a context with an active span using the test tracer
|
||||
tracer := tp.Tracer("test")
|
||||
ctx, span := tracer.Start(t.Context(), "test-span")
|
||||
defer span.End()
|
||||
|
||||
// Setup context with test-specific data
|
||||
ctx = tc.setupContext(ctx)
|
||||
|
||||
// Create and handle an error
|
||||
err := New(ctx, tc.errorMessage)
|
||||
Handle(ctx, err)
|
||||
|
||||
// Force span to end so we can check recorded data
|
||||
span.End()
|
||||
|
||||
// Check that the exception event was created
|
||||
spans := sr.Ended()
|
||||
require.Len(t, spans, 1)
|
||||
|
||||
// Find the exception event
|
||||
events := spans[0].Events()
|
||||
var exceptionEvent *trace.Event
|
||||
for i := range events {
|
||||
if events[i].Name == "exception" {
|
||||
exceptionEvent = &events[i]
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, exceptionEvent, "Expected to find an exception event")
|
||||
|
||||
// Check all expected attributes are present
|
||||
attributes := make(map[string]any)
|
||||
for _, attr := range exceptionEvent.Attributes {
|
||||
switch attr.Key {
|
||||
case "user.id", "host.id":
|
||||
attributes[string(attr.Key)] = attr.Value.AsInt64()
|
||||
default:
|
||||
attributes[string(attr.Key)] = attr.Value.AsString()
|
||||
}
|
||||
}
|
||||
|
||||
// Always check for stack trace
|
||||
stackTrace, ok := attributes["exception.stacktrace"].(string)
|
||||
assert.True(t, ok, "Expected exception.stacktrace attribute")
|
||||
assert.Contains(t, stackTrace, "TestHandleSendsContextToOTEL", "Stack trace should contain test function name")
|
||||
assert.True(t, strings.Contains(stackTrace, "\n"), "Stack trace should be formatted with newlines")
|
||||
|
||||
// Check for exception message and type
|
||||
assert.Equal(t, tc.errorMessage, attributes["exception.message"])
|
||||
assert.Equal(t, "*ctxerr.FleetError", attributes["exception.type"])
|
||||
|
||||
// Check test-specific expected attributes
|
||||
for expectedKey, expectedValue := range tc.expectedAttrs {
|
||||
actualValue, found := attributes[expectedKey]
|
||||
assert.True(t, found, "Expected to find attribute %s", expectedKey)
|
||||
assert.Equal(t, expectedValue, actualValue, "Attribute %s should match", expectedKey)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -3,6 +3,7 @@ package service
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"os"
|
||||
@@ -29,6 +30,7 @@ import (
|
||||
"github.com/fleetdm/fleet/v4/server/service/middleware/endpoint_utils"
|
||||
"github.com/fleetdm/fleet/v4/server/service/middleware/log"
|
||||
"github.com/fleetdm/fleet/v4/server/service/middleware/mdmconfigured"
|
||||
"github.com/fleetdm/fleet/v4/server/service/middleware/otel"
|
||||
"github.com/fleetdm/fleet/v4/server/service/middleware/ratelimit"
|
||||
kithttp "github.com/go-kit/kit/transport/http"
|
||||
kitlog "github.com/go-kit/log"
|
||||
@@ -1174,14 +1176,15 @@ func RegisterAppleMDMProtocolServices(
|
||||
ddmService nanomdm_service.DeclarativeManagement,
|
||||
profileService nanomdm_service.ProfileService,
|
||||
serverURLPrefix string,
|
||||
fleetConfig config.FleetConfig,
|
||||
) error {
|
||||
if err := registerSCEP(mux, scepConfig, scepStorage, mdmStorage, logger); err != nil {
|
||||
if err := registerSCEP(mux, scepConfig, scepStorage, mdmStorage, logger, fleetConfig); err != nil {
|
||||
return fmt.Errorf("scep: %w", err)
|
||||
}
|
||||
if err := registerMDM(mux, mdmStorage, checkinAndCommandService, ddmService, profileService, logger); err != nil {
|
||||
if err := registerMDM(mux, mdmStorage, checkinAndCommandService, ddmService, profileService, logger, fleetConfig); err != nil {
|
||||
return fmt.Errorf("mdm: %w", err)
|
||||
}
|
||||
if err := registerMDMServiceDiscovery(mux, logger, serverURLPrefix); err != nil {
|
||||
if err := registerMDMServiceDiscovery(mux, logger, serverURLPrefix, fleetConfig); err != nil {
|
||||
return fmt.Errorf("service discovery: %w", err)
|
||||
}
|
||||
return nil
|
||||
@@ -1191,6 +1194,7 @@ func registerMDMServiceDiscovery(
|
||||
mux *http.ServeMux,
|
||||
logger kitlog.Logger,
|
||||
serverURLPrefix string,
|
||||
fleetConfig config.FleetConfig,
|
||||
) error {
|
||||
serviceDiscoveryLogger := kitlog.With(logger, "component", "mdm-apple-service-discovery")
|
||||
fullMDMEnrollmentURL := fmt.Sprintf("%s%s", serverURLPrefix, apple_mdm.AccountDrivenEnrollPath)
|
||||
@@ -1204,7 +1208,7 @@ func registerMDMServiceDiscovery(
|
||||
http.Error(w, "Internal Server Error", http.StatusInternalServerError)
|
||||
}
|
||||
})
|
||||
mux.Handle(apple_mdm.ServiceDiscoveryPath, serviceDiscoveryHandler)
|
||||
mux.Handle(apple_mdm.ServiceDiscoveryPath, otel.WrapHandler(serviceDiscoveryHandler, apple_mdm.ServiceDiscoveryPath, fleetConfig))
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -1216,6 +1220,7 @@ func registerSCEP(
|
||||
scepStorage scep_depot.Depot,
|
||||
mdmStorage fleet.MDMAppleStore,
|
||||
logger kitlog.Logger,
|
||||
fleetConfig config.FleetConfig,
|
||||
) error {
|
||||
var signer scepserver.CSRSignerContext = scepserver.SignCSRAdapter(scep_depot.NewSigner(
|
||||
scepStorage,
|
||||
@@ -1240,7 +1245,7 @@ func registerSCEP(
|
||||
e.GetEndpoint = scepserver.EndpointLoggingMiddleware(scepLogger)(e.GetEndpoint)
|
||||
e.PostEndpoint = scepserver.EndpointLoggingMiddleware(scepLogger)(e.PostEndpoint)
|
||||
scepHandler := scepserver.MakeHTTPHandler(e, scepService, scepLogger)
|
||||
mux.Handle(apple_mdm.SCEPPath, scepHandler)
|
||||
mux.Handle(apple_mdm.SCEPPath, otel.WrapHandler(scepHandler, apple_mdm.SCEPPath, fleetConfig))
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -1249,7 +1254,11 @@ func RegisterSCEPProxy(
|
||||
ds fleet.Datastore,
|
||||
logger kitlog.Logger,
|
||||
timeout *time.Duration,
|
||||
fleetConfig *config.FleetConfig,
|
||||
) error {
|
||||
if fleetConfig == nil {
|
||||
return errors.New("fleet config is nil")
|
||||
}
|
||||
scepService := eeservice.NewSCEPProxyService(
|
||||
ds,
|
||||
kitlog.With(logger, "component", "scep-proxy-service"),
|
||||
@@ -1260,6 +1269,8 @@ func RegisterSCEPProxy(
|
||||
e.GetEndpoint = scepserver.EndpointLoggingMiddleware(scepLogger)(e.GetEndpoint)
|
||||
e.PostEndpoint = scepserver.EndpointLoggingMiddleware(scepLogger)(e.PostEndpoint)
|
||||
scepHandler := scepserver.MakeHTTPHandlerWithIdentifier(e, apple_mdm.SCEPProxyPath, scepLogger)
|
||||
// Not using OTEL dynamic wrapper so as not to expose {identifier} in the span name
|
||||
scepHandler = otel.WrapHandler(scepHandler, apple_mdm.SCEPProxyPath, *fleetConfig)
|
||||
rootMux.Handle(apple_mdm.SCEPProxyPath, scepHandler)
|
||||
return nil
|
||||
}
|
||||
@@ -1298,6 +1309,7 @@ func registerMDM(
|
||||
ddmService nanomdm_service.DeclarativeManagement,
|
||||
profileService nanomdm_service.ProfileService,
|
||||
logger kitlog.Logger,
|
||||
fleetConfig config.FleetConfig,
|
||||
) error {
|
||||
certVerifier := mdmcrypto.NewSCEPVerifier(mdmStorage)
|
||||
mdmLogger := NewNanoMDMLogger(kitlog.With(logger, "component", "http-mdm-apple-mdm"))
|
||||
@@ -1329,7 +1341,7 @@ func registerMDM(
|
||||
}
|
||||
mdmHandler = httpmdm.CertExtractMdmSignatureMiddleware(mdmHandler, httpmdm.MdmSignatureVerifierFunc(cryptoutil.VerifyMdmSignature),
|
||||
httpmdm.SigLogWithLogger(mdmLogger.With("handler", "cert-extract")))
|
||||
mux.Handle(apple_mdm.MDMPath, mdmHandler)
|
||||
mux.Handle(apple_mdm.MDMPath, otel.WrapHandler(mdmHandler, apple_mdm.MDMPath, fleetConfig))
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
package otel
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/server/config"
|
||||
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
|
||||
)
|
||||
|
||||
// WrapHandler wraps an HTTP handler with OpenTelemetry instrumentation for a fixed route.
|
||||
// It creates spans named as "{method} {route}" (e.g., "GET /healthz").
|
||||
func WrapHandler(handler http.Handler, route string, config config.FleetConfig) http.Handler {
|
||||
if config.Logging.TracingEnabled && config.Logging.TracingType == "opentelemetry" {
|
||||
// Wrap with OTEL handler to create properly named spans: "{method} {route}"
|
||||
return otelhttp.NewHandler(
|
||||
otelhttp.WithRouteTag(route, handler),
|
||||
"", // Empty operation name - will be set by span name formatter
|
||||
otelhttp.WithSpanNameFormatter(func(operation string, r *http.Request) string {
|
||||
return r.Method + " " + route
|
||||
}),
|
||||
)
|
||||
}
|
||||
return handler
|
||||
}
|
||||
|
||||
// WrapHandlerDynamic wraps an HTTP handler with OpenTelemetry instrumentation using dynamic routes.
|
||||
// It creates spans based on the actual request path (e.g., "GET /assets/app.js").
|
||||
func WrapHandlerDynamic(handler http.Handler, config config.FleetConfig) http.Handler {
|
||||
if config.Logging.TracingEnabled && config.Logging.TracingType == "opentelemetry" {
|
||||
// Create a wrapper that instruments each request with its actual path
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
// Use the actual request path as the route
|
||||
route := r.URL.Path
|
||||
instrumentedHandler := otelhttp.NewHandler(
|
||||
otelhttp.WithRouteTag(route, handler),
|
||||
"", // Empty operation name - will be set by span name formatter
|
||||
otelhttp.WithSpanNameFormatter(func(operation string, req *http.Request) string {
|
||||
return req.Method + " " + route
|
||||
}),
|
||||
)
|
||||
instrumentedHandler.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
return handler
|
||||
}
|
||||
@@ -441,6 +441,7 @@ func RunServerForTestsWithServiceWithDS(t *testing.T, ctx context.Context, ds fl
|
||||
},
|
||||
commander,
|
||||
"https://test-url.com",
|
||||
cfg,
|
||||
)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
@@ -458,6 +459,7 @@ func RunServerForTestsWithServiceWithDS(t *testing.T, ctx context.Context, ds fl
|
||||
ds,
|
||||
logger,
|
||||
timeout,
|
||||
&cfg,
|
||||
)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
@@ -479,7 +481,7 @@ func RunServerForTestsWithServiceWithDS(t *testing.T, ctx context.Context, ds fl
|
||||
extra = append(extra, WithLoginRateLimit(throttled.PerMin(1000)))
|
||||
|
||||
if len(opts) > 0 && opts[0].HostIdentity != nil {
|
||||
require.NoError(t, hostidentity.RegisterSCEP(rootMux, opts[0].HostIdentity.SCEPStorage, ds, logger))
|
||||
require.NoError(t, hostidentity.RegisterSCEP(rootMux, opts[0].HostIdentity.SCEPStorage, ds, logger, &cfg))
|
||||
var httpSigVerifier func(http.Handler) http.Handler
|
||||
httpSigVerifier, err := httpsig.Middleware(ds, opts[0].HostIdentity.RequireHTTPMessageSignature, kitlog.With(logger, "component", "http-sig-verifier"))
|
||||
require.NoError(t, err)
|
||||
@@ -497,7 +499,7 @@ func RunServerForTestsWithServiceWithDS(t *testing.T, ctx context.Context, ds fl
|
||||
rootMux.Handle("/enroll", ServeEndUserEnrollOTA(svc, "", ds, logger))
|
||||
|
||||
if len(opts) > 0 && opts[0].EnableSCIM {
|
||||
require.NoError(t, scim.RegisterSCIM(rootMux, ds, svc, logger))
|
||||
require.NoError(t, scim.RegisterSCIM(rootMux, ds, svc, logger, &cfg))
|
||||
rootMux.Handle("/api/v1/fleet/scim/details", apiHandler)
|
||||
rootMux.Handle("/api/latest/fleet/scim/details", apiHandler)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user