CORE/FFI/GO - SpanFromContext implementation#4507

Merged

affonsov merged 12 commits intomainfrom

go/affonsov-otel-span-from-context

Aug 8, 2025

Collaborator

affonsov commented Jul 29, 2025

OpenTelemetry Span Context Attachment Enhancement

Overview

This PR significantly enhances the OpenTelemetry integration by implementing comprehensive span context attachment functionality, enabling proper parent-child span relationships and improved tracing capabilities across the Glide client library.

Key Features Added

🔧 FFI Layer Enhancements

New FFI Functions:
- create_named_otel_span() - Creates spans with custom names for user operations
- create_otel_span_with_parent() - Creates child spans with parent-child relationships
- create_batch_otel_span_with_parent() - Creates batch spans with parent context
Enhanced Error Handling:
- Input validation with detailed logging
- Graceful fallback mechanisms for invalid parent spans
- Memory safety improvements with pointer validation
- Panic recovery in span cleanup operations

🎯 Go Client API Improvements

Public Span Management APIs:
- CreateSpan(name string) - Create named spans for user operations
- EndSpan(spanPtr uint64) - Properly clean up spans
- WithSpan(ctx, spanPtr) - Attach span context to Go contexts
- DefaultSpanFromContext(ctx) - Extract span pointers from contexts
Context Integration:
- Seamless integration with Go's context.Context
- Configurable span extraction strategies
- Thread-safe span context management

🔗 Span Hierarchy Support

Parent-Child Relationships: Commands executed within a span context automatically become child spans
Batch Operations: Batch commands properly inherit parent span context
Mixed Scenarios: Support for operations with and without parent context in the same session

Backward Compatibility

All existing OpenTelemetry functionality remains unchanged
New features are opt-in and don't affect existing code
Graceful degradation when OpenTelemetry is not initialized

Issue link

This Pull Request is linked to issue (URL): #4243

Checklist

Before submitting the PR make sure the following are checked:

This Pull Request is related to one issue.
Commit message has a detailed description of what changed and why.
Tests are added or updated.
CHANGELOG.md and documentation files are updated.
Destination branch is correct - main or release
Create merge commit if merging release branch into main, squash otherwise.

affonsov requested a review from a team as a code owner

July 29, 2025 16:58

affonsov had a problem deploying to AWS_ACTIONS

July 29, 2025 16:59

— with

GitHub Actions Error

affonsov had a problem deploying to AWS_ACTIONS

July 29, 2025 18:17

— with

GitHub Actions Error

jonathanl-bq reviewed

View reviewed changes

glide-core/telemetry/src/open_telemetry.rs Outdated Show resolved Hide resolved

jonathanl-bq reviewed

View reviewed changes

glide-core/telemetry/src/open_telemetry.rs Outdated Show resolved Hide resolved

yipin-chen requested review from jamesx-improving and prateek-kumar-improving

July 29, 2025 22:39

affonsov had a problem deploying to AWS_ACTIONS

August 1, 2025 17:33

— with

GitHub Actions Error

yipin-chen added go 🏃 Core changes 🪐 Telemetry 📺 labels

prateek-kumar-improving reviewed

View reviewed changes

go/opentelemetry.go Show resolved Hide resolved

prateek-kumar-improving reviewed

View reviewed changes

go/opentelemetry.go Show resolved Hide resolved

prateek-kumar-improving reviewed

View reviewed changes

go/opentelemetry.go Outdated Show resolved Hide resolved

prateek-kumar-improving reviewed

View reviewed changes

go/opentelemetry.go Outdated Show resolved Hide resolved

affonsov force-pushed the go/affonsov-otel-span-from-context branch from 88e262a to 226094f Compare

August 6, 2025 04:01

affonsov had a problem deploying to AWS_ACTIONS

August 6, 2025 04:01

— with

GitHub Actions Error

affonsov force-pushed the go/affonsov-otel-span-from-context branch from 226094f to 422919b Compare

August 6, 2025 04:05

affonsov had a problem deploying to AWS_ACTIONS

August 6, 2025 04:05

— with

GitHub Actions Error

affonsov had a problem deploying to AWS_ACTIONS

August 6, 2025 05:42

— with

GitHub Actions Error

affonsov had a problem deploying to AWS_ACTIONS

August 6, 2025 06:31

— with

GitHub Actions Error

affonsov had a problem deploying to AWS_ACTIONS

August 6, 2025 06:41

— with

GitHub Actions Error

jonathanl-bq reviewed

View reviewed changes

Contributor

jonathanl-bq left a comment

Can you add MIRI tests as well? We need coverage for the new functions you added to the FFI layer.

ffi/src/lib.rs Outdated Show resolved Hide resolved

ffi/src/lib.rs Outdated Show resolved Hide resolved

affonsov added 7 commits

August 6, 2025 13:53


          Core: Added span pointer conversion and validation

456fc42

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>


          ffi: The changes add extensive test coverage for the OpenTelemetry FF…

6a3fd1f

…I functions including:

-Enhanced error handling tests for invalid inputs (null pointers, oversized names, invalid request types)
-Comprehensive integration tests covering span creation, parent-child relationships, and memory safety
-Batch span functionality tests with hierarchy validation
-Concurrent access testing and boundary condition validation
-Memory safety tests for span cleanup operations

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>


          go: New OpenTelemetry functionality

23464d4

- integration tests
- Unit tests
- Base client modifications to support the new parent/child span

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>


          fixing ffi tests failing on github actions

86f372d

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>


          Change glide-core function to be unsafe

b0510f7

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>


          Simplify OpenTelemetry span context API and improve miri test mocks

92e56d1

- Simplify SpanFromContext function signature from (uint64, bool) to uint64
  - Remove redundant 'found' boolean return value
  - Functions now return 0 to indicate no parent span available
  - Update DefaultSpanFromContext and all related implementations
  - Improve error handling and panic recovery logic

- Add mock-logger-core package for miri tests
  - Create no-op implementations of log functions (error, warn, debug, info)
  - Add proper Cargo.toml configuration and dependency integration
  - Ensure miri tests can run without complex logging infrastructure

- Enhance mock-telemetry for better miri test coverage
  - Implement Display trait for TraceError
  - Add concrete implementations for GlideOpenTelemetry methods
  - Replace todo!() placeholders with working mock implementations
  - Add span_from_pointer method for completeness

- Update Go client span extraction logic
  - Simplify parent span detection from context
  - Remove unnecessary tuple unpacking in executeCommandWithRoute and executeBatch
  - Maintain backward compatibility while improving code clarity

- Add comprehensive OpenTelemetry examples and documentation
  - Create opentelemetry_examples_test.go with runnable examples
  - Update examples.md to reference new OpenTelemetry examples
  - Move extensive documentation from inline comments to dedicated examples
  - Provide clear usage patterns for span context management

This change improves the developer experience by simplifying the OpenTelemetry
integration API while maintaining full functionality and backward compatibility.
The miri test improvements ensure better test coverage in memory-safe environments.

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>


          fix lint

b1a7dbf

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

affonsov added 3 commits

August 6, 2025 13:53


          simplify OpenTelemetry examples and improve span relationship demo

d2a1735

- Remove ExampleWithSpan and ExampleDefaultSpanFromContext functions
- Rename ExampleOpenTelemetry_CreateSpan to ExampleOpenTelemetry
- Focus example on demonstrating parent-child span relationships
- Add clearer comments explaining span context usage with Valkey commands
- Clean up formatting and remove redundant content

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>


          fix linter

3e3e78a

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>


          - Add proper RequestType enum with concrete command variants (Get, Se…

a467e34

…t, Del, etc.)

- Implement enhanced Cmd struct with command_bytes field and default implementation
- Add comprehensive Miri tests for all OTEL span creation functions
- Fix function signatures to properly mark unsafe extern "C" functions
- Improve error handling and fallback behavior for invalid parent spans
- Add extensive test coverage for span hierarchies and concurrent access
- Clean up code formatting and organize imports consistently
- Strengthen memory safety validation in FFI boundary functions

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

affonsov force-pushed the go/affonsov-otel-span-from-context branch from bdcdbfc to a467e34 Compare

August 6, 2025 22:04

affonsov had a problem deploying to AWS_ACTIONS

August 6, 2025 22:04

— with

GitHub Actions Error


          ffi lint fixes

d8b46db

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

affonsov had a problem deploying to AWS_ACTIONS

August 6, 2025 22:07

— with

GitHub Actions Error

prateek-kumar-improving reviewed

View reviewed changes

go/base_client.go Outdated Show resolved Hide resolved

jonathanl-bq approved these changes

View reviewed changes

ffi/src/lib.rs Show resolved Hide resolved


          removed (current behavior) from the comment

a0c16e8

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

affonsov had a problem deploying to AWS_ACTIONS

August 7, 2025 00:29

— with

GitHub Actions Error

affonsov requested a deployment to AWS_ACTIONS

August 7, 2025 17:13

— with

GitHub Actions Waiting

prateek-kumar-improving approved these changes

View reviewed changes

affonsov merged commit c7db02f into main

53 of 59 checks passed

Contributor

jonathanl-bq commented Aug 8, 2025

Changelog is updated for this PR in #4567

zarkash-aws pushed a commit to zarkash-aws/valkey-glide that referenced this pull request


          CORE/FFI/GO - SpanFromContext implementation (valkey-io#4507)

5be2e7f

* Core: Added span pointer conversion and validation

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* ffi: The changes add extensive test coverage for the OpenTelemetry FFI functions including:

-Enhanced error handling tests for invalid inputs (null pointers, oversized names, invalid request types)
-Comprehensive integration tests covering span creation, parent-child relationships, and memory safety
-Batch span functionality tests with hierarchy validation
-Concurrent access testing and boundary condition validation
-Memory safety tests for span cleanup operations

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* go: New OpenTelemetry functionality

- integration tests
- Unit tests
- Base client modifications to support the new parent/child span

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* fixing ffi tests failing on github actions

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* Change glide-core function to be unsafe

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* Simplify OpenTelemetry span context API and improve miri test mocks

- Simplify SpanFromContext function signature from (uint64, bool) to uint64
  - Remove redundant 'found' boolean return value
  - Functions now return 0 to indicate no parent span available
  - Update DefaultSpanFromContext and all related implementations
  - Improve error handling and panic recovery logic

- Add mock-logger-core package for miri tests
  - Create no-op implementations of log functions (error, warn, debug, info)
  - Add proper Cargo.toml configuration and dependency integration
  - Ensure miri tests can run without complex logging infrastructure

- Enhance mock-telemetry for better miri test coverage
  - Implement Display trait for TraceError
  - Add concrete implementations for GlideOpenTelemetry methods
  - Replace todo!() placeholders with working mock implementations
  - Add span_from_pointer method for completeness

- Update Go client span extraction logic
  - Simplify parent span detection from context
  - Remove unnecessary tuple unpacking in executeCommandWithRoute and executeBatch
  - Maintain backward compatibility while improving code clarity

- Add comprehensive OpenTelemetry examples and documentation
  - Create opentelemetry_examples_test.go with runnable examples
  - Update examples.md to reference new OpenTelemetry examples
  - Move extensive documentation from inline comments to dedicated examples
  - Provide clear usage patterns for span context management

This change improves the developer experience by simplifying the OpenTelemetry
integration API while maintaining full functionality and backward compatibility.
The miri test improvements ensure better test coverage in memory-safe environments.

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* fix lint

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* simplify OpenTelemetry examples and improve span relationship demo

- Remove ExampleWithSpan and ExampleDefaultSpanFromContext functions
- Rename ExampleOpenTelemetry_CreateSpan to ExampleOpenTelemetry
- Focus example on demonstrating parent-child span relationships
- Add clearer comments explaining span context usage with Valkey commands
- Clean up formatting and remove redundant content

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* fix linter

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* - Add proper RequestType enum with concrete command variants (Get, Set, Del, etc.)
- Implement enhanced Cmd struct with command_bytes field and default implementation
- Add comprehensive Miri tests for all OTEL span creation functions
- Fix function signatures to properly mark unsafe extern "C" functions
- Improve error handling and fallback behavior for invalid parent spans
- Add extensive test coverage for span hierarchies and concurrent access
- Clean up code formatting and organize imports consistently
- Strengthen memory safety validation in FFI boundary functions

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* ffi lint fixes

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* removed (current behavior) from the comment

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

---------

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

alexr-bq pushed a commit that referenced this pull request


          CORE/FFI/GO - SpanFromContext implementation (#4507)

a3ce478

* Core: Added span pointer conversion and validation

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* ffi: The changes add extensive test coverage for the OpenTelemetry FFI functions including:

-Enhanced error handling tests for invalid inputs (null pointers, oversized names, invalid request types)
-Comprehensive integration tests covering span creation, parent-child relationships, and memory safety
-Batch span functionality tests with hierarchy validation
-Concurrent access testing and boundary condition validation
-Memory safety tests for span cleanup operations

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* go: New OpenTelemetry functionality

- integration tests
- Unit tests
- Base client modifications to support the new parent/child span

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* fixing ffi tests failing on github actions

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* Change glide-core function to be unsafe

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* Simplify OpenTelemetry span context API and improve miri test mocks

- Simplify SpanFromContext function signature from (uint64, bool) to uint64
  - Remove redundant 'found' boolean return value
  - Functions now return 0 to indicate no parent span available
  - Update DefaultSpanFromContext and all related implementations
  - Improve error handling and panic recovery logic

- Add mock-logger-core package for miri tests
  - Create no-op implementations of log functions (error, warn, debug, info)
  - Add proper Cargo.toml configuration and dependency integration
  - Ensure miri tests can run without complex logging infrastructure

- Enhance mock-telemetry for better miri test coverage
  - Implement Display trait for TraceError
  - Add concrete implementations for GlideOpenTelemetry methods
  - Replace todo!() placeholders with working mock implementations
  - Add span_from_pointer method for completeness

- Update Go client span extraction logic
  - Simplify parent span detection from context
  - Remove unnecessary tuple unpacking in executeCommandWithRoute and executeBatch
  - Maintain backward compatibility while improving code clarity

- Add comprehensive OpenTelemetry examples and documentation
  - Create opentelemetry_examples_test.go with runnable examples
  - Update examples.md to reference new OpenTelemetry examples
  - Move extensive documentation from inline comments to dedicated examples
  - Provide clear usage patterns for span context management

This change improves the developer experience by simplifying the OpenTelemetry
integration API while maintaining full functionality and backward compatibility.
The miri test improvements ensure better test coverage in memory-safe environments.

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* fix lint

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* simplify OpenTelemetry examples and improve span relationship demo

- Remove ExampleWithSpan and ExampleDefaultSpanFromContext functions
- Rename ExampleOpenTelemetry_CreateSpan to ExampleOpenTelemetry
- Focus example on demonstrating parent-child span relationships
- Add clearer comments explaining span context usage with Valkey commands
- Clean up formatting and remove redundant content

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* fix linter

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* - Add proper RequestType enum with concrete command variants (Get, Set, Del, etc.)
- Implement enhanced Cmd struct with command_bytes field and default implementation
- Add comprehensive Miri tests for all OTEL span creation functions
- Fix function signatures to properly mark unsafe extern "C" functions
- Improve error handling and fallback behavior for invalid parent spans
- Add extensive test coverage for span hierarchies and concurrent access
- Clean up code formatting and organize imports consistently
- Strengthen memory safety validation in FFI boundary functions

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* ffi lint fixes

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* removed (current behavior) from the comment

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

---------

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>

gavinhailey mentioned this pull request

(Node.js): Add OpenTelemetry existing parent span support #4655

Open

2 tasks

alexr-bq pushed a commit that referenced this pull request


          CORE/FFI/GO - SpanFromContext implementation (#4507)

f6c3e4b

* Core: Added span pointer conversion and validation

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* ffi: The changes add extensive test coverage for the OpenTelemetry FFI functions including:

-Enhanced error handling tests for invalid inputs (null pointers, oversized names, invalid request types)
-Comprehensive integration tests covering span creation, parent-child relationships, and memory safety
-Batch span functionality tests with hierarchy validation
-Concurrent access testing and boundary condition validation
-Memory safety tests for span cleanup operations

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* go: New OpenTelemetry functionality

- integration tests
- Unit tests
- Base client modifications to support the new parent/child span

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* fixing ffi tests failing on github actions

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* Change glide-core function to be unsafe

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* Simplify OpenTelemetry span context API and improve miri test mocks

- Simplify SpanFromContext function signature from (uint64, bool) to uint64
  - Remove redundant 'found' boolean return value
  - Functions now return 0 to indicate no parent span available
  - Update DefaultSpanFromContext and all related implementations
  - Improve error handling and panic recovery logic

- Add mock-logger-core package for miri tests
  - Create no-op implementations of log functions (error, warn, debug, info)
  - Add proper Cargo.toml configuration and dependency integration
  - Ensure miri tests can run without complex logging infrastructure

- Enhance mock-telemetry for better miri test coverage
  - Implement Display trait for TraceError
  - Add concrete implementations for GlideOpenTelemetry methods
  - Replace todo!() placeholders with working mock implementations
  - Add span_from_pointer method for completeness

- Update Go client span extraction logic
  - Simplify parent span detection from context
  - Remove unnecessary tuple unpacking in executeCommandWithRoute and executeBatch
  - Maintain backward compatibility while improving code clarity

- Add comprehensive OpenTelemetry examples and documentation
  - Create opentelemetry_examples_test.go with runnable examples
  - Update examples.md to reference new OpenTelemetry examples
  - Move extensive documentation from inline comments to dedicated examples
  - Provide clear usage patterns for span context management

This change improves the developer experience by simplifying the OpenTelemetry
integration API while maintaining full functionality and backward compatibility.
The miri test improvements ensure better test coverage in memory-safe environments.

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* fix lint

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* simplify OpenTelemetry examples and improve span relationship demo

- Remove ExampleWithSpan and ExampleDefaultSpanFromContext functions
- Rename ExampleOpenTelemetry_CreateSpan to ExampleOpenTelemetry
- Focus example on demonstrating parent-child span relationships
- Add clearer comments explaining span context usage with Valkey commands
- Clean up formatting and remove redundant content

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* fix linter

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* - Add proper RequestType enum with concrete command variants (Get, Set, Del, etc.)
- Implement enhanced Cmd struct with command_bytes field and default implementation
- Add comprehensive Miri tests for all OTEL span creation functions
- Fix function signatures to properly mark unsafe extern "C" functions
- Improve error handling and fallback behavior for invalid parent spans
- Add extensive test coverage for span hierarchies and concurrent access
- Clean up code formatting and organize imports consistently
- Strengthen memory safety validation in FFI boundary functions

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* ffi lint fixes

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

* removed (current behavior) from the comment

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>

---------

Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>

affonsov deleted the go/affonsov-otel-span-from-context branch

September 15, 2025 15:20

jamesx-improving mentioned this pull request

[Inquiry] OTEL for Go setup #4730

Open

mahendrabishnoi2 mentioned this pull request

(Python): Add OpenTelemetry existing parent span support #4992

Open

2 tasks

This was referenced Dec 12, 2025

(Java): Add OpenTelemetry existing parent span support #5061

Open

Add OpenTelemetry existing parent span support valkey-io/valkey-glide-csharp#162

Open

Add OpenTelemetry existing parent span support valkey-io/valkey-glide-php#95

Open

naoki-tateyama mentioned this pull request

(OpenTelemetry): Spans created by GLIDE are isolated from application trace context #5415

Closed

2 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Core changes 🪐 go 🏃 Telemetry 📺