From ea5573db38caaf1de1b08dcc23689499c86e8134 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Tue, 11 Jun 2024 11:29:02 -0700 Subject: [PATCH] checkpoint --- .../aws/aws-lambda-go/swolambda/swolambda.go | 19 ++++-- internal/config/config.go | 9 +-- internal/entryspans/entryspans.go | 58 +++++++++++++++---- internal/entryspans/entryspans_test.go | 10 +++- internal/exporter/exporter.go | 4 +- internal/metrics/otel.go | 9 +-- internal/metrics/registry.go | 4 +- internal/{utils/otel.go => txn/txn.go} | 14 +++-- .../{utils/otel_test.go => txn/txn_test.go} | 2 +- swo/agent.go | 1 - 10 files changed, 95 insertions(+), 35 deletions(-) rename internal/{utils/otel.go => txn/txn.go} (91%) rename internal/{utils/otel_test.go => txn/txn_test.go} (99%) diff --git a/instrumentation/github.com/aws/aws-lambda-go/swolambda/swolambda.go b/instrumentation/github.com/aws/aws-lambda-go/swolambda/swolambda.go index 816da3b2..4f846d3c 100644 --- a/instrumentation/github.com/aws/aws-lambda-go/swolambda/swolambda.go +++ b/instrumentation/github.com/aws/aws-lambda-go/swolambda/swolambda.go @@ -18,6 +18,7 @@ import ( "context" "github.com/aws/aws-lambda-go/lambda" "github.com/aws/aws-lambda-go/lambdacontext" + "github.com/solarwinds/apm-go/internal/config" "github.com/solarwinds/apm-go/internal/log" "github.com/solarwinds/apm-go/swo" "go.opentelemetry.io/otel" @@ -25,6 +26,7 @@ import ( semconv "go.opentelemetry.io/otel/semconv/v1.24.0" "go.opentelemetry.io/otel/trace" "os" + "strings" "sync" ) @@ -33,7 +35,9 @@ var tracer trace.Tracer var once sync.Once type wrappedHandler struct { - base lambda.Handler + base lambda.Handler + fnName string + txnName string } var _ lambda.Handler = &wrappedHandler{} @@ -45,8 +49,8 @@ func (w *wrappedHandler) Invoke(ctx context.Context, payload []byte) ([]byte, er } else if lc != nil { attrs = append(attrs, semconv.FaaSInvocationID(lc.AwsRequestID)) } - name := os.Getenv("AWS_LAMBDA_FUNCTION_NAME") - ctx, span := tracer.Start(ctx, name, trace.WithSpanKind(trace.SpanKindServer), trace.WithAttributes(attrs...)) + attrs = append(attrs, attribute.String("sw.transaction", w.txnName)) + ctx, span := tracer.Start(ctx, w.fnName, trace.WithSpanKind(trace.SpanKindServer), trace.WithAttributes(attrs...)) defer func() { span.End() if flusher != nil { @@ -66,7 +70,14 @@ func WrapHandler(f interface{}) lambda.Handler { } tracer = otel.GetTracerProvider().Tracer("swolambda") }) + fnName := os.Getenv("AWS_LAMBDA_FUNCTION_NAME") + txnName := strings.TrimSpace(config.GetTransactionName()) + if txnName == "" { + txnName = fnName + } return &wrappedHandler{ - base: lambda.NewHandler(f), + base: lambda.NewHandler(f), + fnName: fnName, + txnName: txnName, } } diff --git a/internal/config/config.go b/internal/config/config.go index c9ef73d0..cc7fe0c0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + // Package config is responsible for loading the configuration from various // sources, e.g., environment variables, configuration files and user input. // It also accepts dynamic settings from the collector server. @@ -340,8 +341,8 @@ const ( may be different from your setting.` ) -// hasLambdaEnv checks if the AWS Lambda env var is set. -func hasLambdaEnv() bool { +// HasLambdaEnv checks if the AWS Lambda env var is set. +func HasLambdaEnv() bool { return os.Getenv("AWS_LAMBDA_FUNCTION_NAME") != "" && os.Getenv("LAMBDA_TASK_ROOT") != "" } @@ -362,12 +363,12 @@ func (c *Config) validate() error { c.Ec2MetadataTimeout = t } - if c.TransactionName != "" && !hasLambdaEnv() { + if c.TransactionName != "" && !HasLambdaEnv() { log.Info(InvalidEnv("TransactionName", c.TransactionName)) c.TransactionName = getFieldDefaultValue(c, "TransactionName") } - if !hasLambdaEnv() { + if !HasLambdaEnv() { if c.ServiceKey != "" { c.ServiceKey = ToServiceKey(c.ServiceKey) if ok := IsValidServiceKey(c.ServiceKey); !ok { diff --git a/internal/entryspans/entryspans.go b/internal/entryspans/entryspans.go index 29a0cce9..26b61c2d 100644 --- a/internal/entryspans/entryspans.go +++ b/internal/entryspans/entryspans.go @@ -17,34 +17,62 @@ package entryspans import ( "fmt" "github.com/pkg/errors" + "github.com/solarwinds/apm-go/internal/config" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/trace" "sync" ) var ( - state = &entrySpans{ - spans: make(map[trace.TraceID][]*entrySpan), - } + state = makeManagerFromEnv() - NotEntrySpan = errors.New("span is not an entry span") + NotEntrySpan = errors.New("span is not an entry span") + CannotSetTransaction = errors.New("cannot set transaction, likely due to lambda enviroment") nullSpanID = trace.SpanID{} nullEntrySpan = &entrySpan{spanId: nullSpanID} ) +type manager interface { + push(tid trace.TraceID, sid trace.SpanID) + delete(tid trace.TraceID, sid trace.SpanID) error + current(tid trace.TraceID) (*entrySpan, bool) + setTransactionName(tid trace.TraceID, name string) error +} + type entrySpan struct { spanId trace.SpanID txnName string } -type entrySpans struct { +type stdManager struct { mut sync.RWMutex spans map[trace.TraceID][]*entrySpan } -func (e *entrySpans) push(tid trace.TraceID, sid trace.SpanID) { +type noopManager struct{} + +func (n noopManager) push(trace.TraceID, trace.SpanID) {} + +func (n noopManager) delete(trace.TraceID, trace.SpanID) error { + return nil +} + +func (n noopManager) current(trace.TraceID) (*entrySpan, bool) { + return nil, false +} + +func (n noopManager) setTransactionName(trace.TraceID, string) error { + return CannotSetTransaction +} + +var ( + _ manager = &stdManager{} + _ manager = &noopManager{} +) + +func (e *stdManager) push(tid trace.TraceID, sid trace.SpanID) { e.mut.Lock() defer e.mut.Unlock() var list []*entrySpan @@ -56,14 +84,14 @@ func (e *entrySpans) push(tid trace.TraceID, sid trace.SpanID) { e.spans[tid] = list } -func (e *entrySpans) current(tid trace.TraceID) (*entrySpan, bool) { +func (e *stdManager) current(tid trace.TraceID) (*entrySpan, bool) { e.mut.Lock() defer e.mut.Unlock() a, ok := e.currentUnsafe(tid) return a, ok } -func (e *entrySpans) currentUnsafe(tid trace.TraceID) (*entrySpan, bool) { +func (e *stdManager) currentUnsafe(tid trace.TraceID) (*entrySpan, bool) { if list, ok := e.spans[tid]; ok { l := len(list) if len(list) == 0 { @@ -85,7 +113,7 @@ func Push(span sdktrace.ReadOnlySpan) error { return nil } -func (e *entrySpans) delete(tid trace.TraceID, sid trace.SpanID) error { +func (e *stdManager) delete(tid trace.TraceID, sid trace.SpanID) error { e.mut.Lock() defer e.mut.Unlock() @@ -125,7 +153,7 @@ func Current(tid trace.TraceID) (trace.SpanID, bool) { return curr.spanId, ok } -func (e *entrySpans) setTransactionName(tid trace.TraceID, name string) error { +func (e *stdManager) setTransactionName(tid trace.TraceID, name string) error { e.mut.Lock() defer e.mut.Unlock() @@ -152,3 +180,13 @@ func IsEntrySpan(span sdktrace.ReadOnlySpan) bool { parent := span.Parent() return !parent.IsValid() || parent.IsRemote() } + +func makeManagerFromEnv() manager { + if config.HasLambdaEnv() { + return &noopManager{} + } else { + return &stdManager{ + spans: make(map[trace.TraceID][]*entrySpan), + } + } +} diff --git a/internal/entryspans/entryspans_test.go b/internal/entryspans/entryspans_test.go index e8823da1..9d45b54c 100644 --- a/internal/entryspans/entryspans_test.go +++ b/internal/entryspans/entryspans_test.go @@ -33,7 +33,7 @@ var ( span4 = trace.SpanID{0x4} ) -func (e *entrySpans) pop(tid trace.TraceID) (trace.SpanID, bool) { +func (e *stdManager) pop(tid trace.TraceID) (trace.SpanID, bool) { e.mut.Lock() defer e.mut.Unlock() @@ -57,6 +57,7 @@ func (e *entrySpans) pop(tid trace.TraceID) (trace.SpanID, bool) { } func TestCurrent(t *testing.T) { + state := state.(*stdManager) sid, ok := Current(traceA) require.False(t, ok) require.False(t, sid.IsValid()) @@ -109,6 +110,7 @@ func TestCurrent(t *testing.T) { } func TestPush(t *testing.T) { + state := state.(*stdManager) var err error tr, teardown := testutils.TracerSetup() defer teardown() @@ -133,7 +135,8 @@ func TestPush(t *testing.T) { func TestSetTransactionName(t *testing.T) { // reset state - state = &entrySpans{spans: make(map[trace.TraceID][]*entrySpan)} + state = &stdManager{spans: make(map[trace.TraceID][]*entrySpan)} + state := state.(*stdManager) err := SetTransactionName(traceA, "foo bar") require.Error(t, err) @@ -174,7 +177,8 @@ func TestSetTransactionName(t *testing.T) { func TestDelete(t *testing.T) { // reset state - state = &entrySpans{spans: make(map[trace.TraceID][]*entrySpan)} + state = &stdManager{spans: make(map[trace.TraceID][]*entrySpan)} + state := state.(*stdManager) err := state.delete(traceA, span1) require.Error(t, err) diff --git a/internal/exporter/exporter.go b/internal/exporter/exporter.go index f950ff8d..e88d13ca 100644 --- a/internal/exporter/exporter.go +++ b/internal/exporter/exporter.go @@ -22,7 +22,7 @@ import ( "github.com/solarwinds/apm-go/internal/log" "github.com/solarwinds/apm-go/internal/reporter" "github.com/solarwinds/apm-go/internal/swotel/semconv" - "github.com/solarwinds/apm-go/internal/utils" + "github.com/solarwinds/apm-go/internal/txn" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" sdktrace "go.opentelemetry.io/otel/sdk/trace" @@ -45,7 +45,7 @@ func (e *exporter) exportSpan(_ context.Context, s sdktrace.ReadOnlySpan) { attribute.String("otel.scope.version", s.InstrumentationScope().Version), }) if entryspans.IsEntrySpan(s) { - evt.AddKV(attribute.String("TransactionName", utils.GetTransactionName(s))) + evt.AddKV(attribute.String("TransactionName", txn.GetTransactionName(s))) // We MUST clear the entry span here. The SpanProcessor only clears entry spans when they are `RecordOnly` if err := entryspans.Delete(s); err != nil { log.Warningf( diff --git a/internal/metrics/otel.go b/internal/metrics/otel.go index 412818ff..daa39bc4 100644 --- a/internal/metrics/otel.go +++ b/internal/metrics/otel.go @@ -17,7 +17,8 @@ package metrics import ( "context" "github.com/solarwinds/apm-go/internal/log" - "github.com/solarwinds/apm-go/internal/utils" + "github.com/solarwinds/apm-go/internal/txn" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/metric" @@ -28,14 +29,13 @@ import ( ) type otelRegistry struct { - meterProvider metric.MeterProvider } func (o *otelRegistry) RecordSpan(span sdktrace.ReadOnlySpan, isAppoptics bool) { // TODO DRY with legacy registry? var attrs = []attribute.KeyValue{ attribute.Bool("sw.is_error", span.Status().Code == codes.Error), - attribute.String("sw.transaction", utils.GetTransactionName(span)), + attribute.String("sw.transaction", txn.GetTransactionName(span)), } for _, attr := range span.Attributes() { // TODO use semconv? @@ -48,7 +48,8 @@ func (o *otelRegistry) RecordSpan(span sdktrace.ReadOnlySpan, isAppoptics bool) } } // TODO service.name? - meter := o.meterProvider.Meter("sw.apm.request.metrics") + + meter := otel.GetMeterProvider().Meter("sw.apm.request.metrics") histo, err := meter.Int64Histogram( "trace.service.response_time", metric.WithExplicitBucketBoundaries(), diff --git a/internal/metrics/registry.go b/internal/metrics/registry.go index 4d25db5d..9dec47c9 100644 --- a/internal/metrics/registry.go +++ b/internal/metrics/registry.go @@ -19,7 +19,7 @@ import ( "github.com/solarwinds/apm-go/internal/bson" "github.com/solarwinds/apm-go/internal/log" "github.com/solarwinds/apm-go/internal/swotel/semconv" - "github.com/solarwinds/apm-go/internal/utils" + "github.com/solarwinds/apm-go/internal/txn" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/sdk/trace" trace2 "go.opentelemetry.io/otel/trace" @@ -192,7 +192,7 @@ func (r *registry) RecordSpan(span trace.ReadOnlySpan, isAppoptics bool) { } swoTags["sw.is_error"] = strconv.FormatBool(isError) - txnName := utils.GetTransactionName(span) + txnName := txn.GetTransactionName(span) swoTags["sw.transaction"] = txnName duration := span.EndTime().Sub(span.StartTime()) diff --git a/internal/utils/otel.go b/internal/txn/txn.go similarity index 91% rename from internal/utils/otel.go rename to internal/txn/txn.go index 9e10eaf0..5571204b 100644 --- a/internal/utils/otel.go +++ b/internal/txn/txn.go @@ -12,9 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -package utils +package txn import ( + "github.com/solarwinds/apm-go/internal/config" "github.com/solarwinds/apm-go/internal/entryspans" "github.com/solarwinds/apm-go/internal/swotel/semconv" "go.opentelemetry.io/otel/attribute" @@ -34,8 +35,13 @@ func GetTransactionName(span sdktrace.ReadOnlySpan) string { } // deriveTransactionName returns transaction name from given span name and attributes, falling back to "unknown" -func deriveTransactionName(name string, attrs []attribute.KeyValue) string { - var httpRoute, httpUrl, txnName = "", "", "" +func deriveTransactionName(name string, attrs []attribute.KeyValue) (txnName string) { + // TODO: add test + if txnName = config.GetTransactionName(); txnName != "" { + return + } + + var httpRoute, httpUrl = "", "" for _, attr := range attrs { if attr.Key == semconv.HTTPRouteKey { httpRoute = attr.Value.AsString() @@ -69,5 +75,5 @@ func deriveTransactionName(name string, attrs []attribute.KeyValue) string { if len(txnName) > 255 { txnName = txnName[:255] } - return txnName + return } diff --git a/internal/utils/otel_test.go b/internal/txn/txn_test.go similarity index 99% rename from internal/utils/otel_test.go rename to internal/txn/txn_test.go index aaf79e73..b579a24c 100644 --- a/internal/utils/otel_test.go +++ b/internal/txn/txn_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package utils +package txn import ( "context" diff --git a/swo/agent.go b/swo/agent.go index 0a557d82..d3cf6dc6 100644 --- a/swo/agent.go +++ b/swo/agent.go @@ -184,7 +184,6 @@ func StartLambda(lambdaLogStreamName string) (Flusher, error) { } else { tpOpts = append(tpOpts, sdktrace.WithSyncer(exprtr)) } - proc := processor.NewInboundMetricsSpanProcessor(registry, false) prop := propagation.NewCompositeTextMapPropagator( &propagation.TraceContext{},