diff --git a/CHANGELOG.md b/CHANGELOG.md index 58a3e0b25..9a8bdd780 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http - Log any failures to close running probes. ([#586](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/586)) - Log explanatory error message on Linux kernel lockdown ([#290](https://github.com/open-telemetry/opentelemetry-go-instrumentation/issues/290)) +- (otelglobal) Fixed case where multiple span.SetAttributes() calls would overwrite existing attributes ([#634](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/634)) ## [v0.10.0-alpha] - 2024-01-03 diff --git a/internal/include/otel_types.h b/internal/include/otel_types.h index cc7eb82c2..8caa5440a 100644 --- a/internal/include/otel_types.h +++ b/internal/include/otel_types.h @@ -92,7 +92,7 @@ static __always_inline bool set_attr_value(otel_attirbute_t *attr, go_otel_attr_ return false; } -static __always_inline void convert_go_otel_attributes(void *attrs_buf, s64 slice_len, otel_attributes_t *enc_attrs) +static __always_inline void convert_go_otel_attributes(void *attrs_buf, u64 slice_len, otel_attributes_t *enc_attrs) { if (attrs_buf == NULL || enc_attrs == NULL){ return; @@ -102,13 +102,16 @@ static __always_inline void convert_go_otel_attributes(void *attrs_buf, s64 slic return; } - s64 num_attrs = slice_len < OTEL_ATTRUBUTE_MAX_COUNT ? slice_len : OTEL_ATTRUBUTE_MAX_COUNT; + u8 num_attrs = slice_len < OTEL_ATTRUBUTE_MAX_COUNT ? slice_len : OTEL_ATTRUBUTE_MAX_COUNT; go_otel_key_value_t *go_attr = (go_otel_key_value_t*)attrs_buf; go_otel_attr_value_t go_attr_value = {0}; struct go_string go_str = {0}; - u8 valid_attrs = 0; + u8 valid_attrs = enc_attrs->valid_attrs; + if (valid_attrs >= OTEL_ATTRUBUTE_MAX_COUNT) { + return; + } - for (u32 go_attr_index = 0; go_attr_index < num_attrs; go_attr_index++) { + for (u8 go_attr_index = 0; go_attr_index < num_attrs; go_attr_index++) { __builtin_memset(&go_attr_value, 0, sizeof(go_otel_attr_value_t)); // Read the value struct bpf_probe_read(&go_attr_value, sizeof(go_otel_attr_value_t), &go_attr[go_attr_index].value); @@ -125,6 +128,13 @@ static __always_inline void convert_go_otel_attributes(void *attrs_buf, s64 slic continue; } + // Need to check valid_attrs otherwise the ebpf verifier thinks it's possible to exceed + // the max register value for a downstream call, even though it's not possible with + // this same check at the end of the loop. + if (valid_attrs >= OTEL_ATTRUBUTE_MAX_COUNT) { + break; + } + if (!get_go_string_from_user_ptr(&go_str, enc_attrs->attrs[valid_attrs].key, OTEL_ATTRIBUTE_KEY_MAX_LEN)) { continue; } @@ -135,6 +145,10 @@ static __always_inline void convert_go_otel_attributes(void *attrs_buf, s64 slic enc_attrs->attrs[valid_attrs].vtype = go_attr_value.vtype; valid_attrs++; + if (valid_attrs >= OTEL_ATTRUBUTE_MAX_COUNT) { + // No more space for attributes + break; + } } enc_attrs->valid_attrs = valid_attrs; diff --git a/internal/test/e2e/otelglobal/main.go b/internal/test/e2e/otelglobal/main.go index 108ac7c79..844233af7 100644 --- a/internal/test/e2e/otelglobal/main.go +++ b/internal/test/e2e/otelglobal/main.go @@ -30,6 +30,7 @@ func innerFunction(ctx context.Context) { defer span.End() span.SetAttributes(attribute.String("inner.key", "inner.value")) + span.SetAttributes(attribute.Bool("cat.on_keyboard", true)) span.SetName("child override") span.SetStatus(codes.Error, "i deleted the prod db sry") } diff --git a/internal/test/e2e/otelglobal/traces.json b/internal/test/e2e/otelglobal/traces.json index 6e722b1a1..3332ab575 100644 --- a/internal/test/e2e/otelglobal/traces.json +++ b/internal/test/e2e/otelglobal/traces.json @@ -56,6 +56,12 @@ "value": { "stringValue": "inner.value" } + }, + { + "key": "cat.on_keyboard", + "value": { + "boolValue": true + } } ], "kind": 3, diff --git a/internal/test/e2e/otelglobal/verify.bats b/internal/test/e2e/otelglobal/verify.bats index a09cabc2a..5a4b37a30 100644 --- a/internal/test/e2e/otelglobal/verify.bats +++ b/internal/test/e2e/otelglobal/verify.bats @@ -24,6 +24,11 @@ SCOPE="go.opentelemetry.io/auto/go.opentelemetry.io/otel/internal/global" assert_equal "$result" '"inner.value"' } +@test "server :: valid bool attribute in child span" { + result=$(span_attributes_for ${SCOPE} | jq "select(.key == \"cat.on_keyboard\").value.boolValue") + assert_equal "$result" 'true' +} + @test "server :: valid bool attribute" { result=$(span_attributes_for ${SCOPE} | jq "select(.key == \"bool_key\").value.boolValue") assert_equal "$result" 'true'