Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce max span attribute size #4335

Merged
merged 13 commits into from
Jan 2, 2025
Prev Previous commit
Next Next commit
add metrics to capture truncated attributes count
  • Loading branch information
ie-pham committed Jan 2, 2025
commit a933707ef069db7fc6e8ebeaa2423cf30dc71e8a
1 change: 0 additions & 1 deletion integration/e2e/limits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ const (
configLimitsQuery = "config-limits-query.yaml"
configLimitsPartialError = "config-limits-partial-success.yaml"
configLimits429 = "config-limits-429.yaml"
configLimitsSpanAttr = "config-limits-span-attr.yaml"
)

func TestLimits(t *testing.T) {
Expand Down
11 changes: 9 additions & 2 deletions modules/distributor/distributor.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ const (
reasonInternalError = "internal_error"
// reasonUnknown indicates a pushByte error at the ingester level not related to GRPC
reasonUnknown = "unknown_error"
// reasonAttrTooLarge indicates that at least one attribute in the span is too large
reasonAttrTooLarge = "attribute_too_large"

distributorRingKey = "distributor"
)
Expand Down Expand Up @@ -115,6 +113,11 @@ var (
Name: "distributor_metrics_generator_clients",
Help: "The current number of metrics-generator clients.",
})
metricAttributesTruncated = promauto.NewCounterVec(prometheus.CounterOpts{
Namespace: "tempo",
Name: "distributor_attributes_truncated_total",
Help: "The total number of proto bytes received per tenant",
}, []string{"tenant"})

statBytesReceived = usagestats.NewCounter("distributor_bytes_received")
statSpansReceived = usagestats.NewCounter("distributor_spans_received")
Expand Down Expand Up @@ -390,6 +393,7 @@ func (d *Distributor) PushTraces(ctx context.Context, traces ptrace.Traces) (*te

if truncatedAttributeCount > 0 {
level.Warn(d.logger).Log("msg", fmt.Sprintf("truncated %d resource/span attributes when adding spans for tenant %s", truncatedAttributeCount, userID))
Copy link
Member

@joe-elliott joe-elliott Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error message doesn't tell us anything that the is not already in the metric. remove? if we add some details that make it worth keeping i think we should consider dropping to debug b/c this could get spammy

metricAttributesTruncated.WithLabelValues(userID).Add(float64(truncatedAttributeCount))
}

err = d.sendToIngestersViaBytes(ctx, userID, spanCount, rebatchedTraces, keys)
Expand Down Expand Up @@ -547,6 +551,8 @@ func requestsByTraceID(batches []*v1.ResourceSpans, userID string, spanCount, ma
for _, ils := range b.ScopeSpans {
for _, span := range ils.Spans {
// check large spans for large attributes
fmt.Printf("span size %d", span.Size())
fmt.Printf("maxSpanAttrSize %d", maxSpanAttrSize)
if span.Size() > maxSpanAttrSize {
processAttributes(span.Attributes, maxSpanAttrSize, &truncatedAttributeCount)
}
Expand Down Expand Up @@ -632,6 +638,7 @@ func processAttributes(attributes []*v1_common.KeyValue, maxAttrSize int, count
switch value := attr.GetValue().Value.(type) {
case *v1_common.AnyValue_StringValue:
if len(value.StringValue) > maxAttrSize {
fmt.Printf("size: %d", len(value.StringValue))
value.StringValue = value.StringValue[:maxAttrSize] + "_truncated"
*count++
}
Expand Down