feat: Add SpanAttributeCustomizer plugin for customizable OpenTelemetry span attributes - #4331
Conversation
17d35be to
7c64f38
Compare
- Add attribute_mapping config field to rename span attribute keys - Apply mapping to both tool spans (ObservabilityService) and plugin spans (PluginManager) - Store mapping in context.global_context.state for runtime access - Add 5 unit tests covering mapping scenarios - Update documentation with mapping examples - Backward compatible: empty/missing mapping = no renaming Closes #4331 Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
- Add attribute_mapping config field to rename span attribute keys - Apply mapping to both tool spans (ObservabilityService) and plugin spans (PluginManager) - Store mapping in context.global_context.state for runtime access - Add 5 unit tests covering mapping scenarios - Update documentation with mapping examples - Backward compatible: empty/missing mapping = no renaming Closes #4331 Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
f0f35b7 to
d286208
Compare
Lang-Akshay
left a comment
There was a problem hiding this comment.
Thanks allot for the PR @vishu-bh . Really appreciate it. Please can you implement following changes.
Security
The plugin operates below the auth/RBAC layer (observability data only). No auth invariants are violated. Security concerns are scoped to PII masking correctness and state handling.
| # | File | Line | Severity | CWE | Description |
|---|---|---|---|---|---|
| 1 | span_attribute_customizer.py:130 |
130 | High | CWE-916/327 | SHA-256 truncated to 16 hex chars, no salt — insufficient for PII masking; rainbow-table reversible for common emails |
| 2 | span_attribute_customizer.py:211 |
211–212 | High | CWE-668 | resource_pre_fetch only sets custom_span_attributes but does not reset remove_span_attributes / span_attribute_mapping; stale tool-invocation state bleeds into resource spans |
| 3 | span_attribute_customizer.py:105 |
105 | Medium | CWE-20 | condition.split("==") raises ValueError silently caught for any condition value containing ==; intended security attributes (audit_required, compliance_level) silently dropped |
| 4 | config_schema.py:40 |
40–58 | Medium | CWE-400 | global_attributes: Dict[str, Any] and conditions[*].add accept unbounded nested objects; OTEL SDKs only accept str/int/float/bool — oversized or wrong-typed values cause silent data loss |
| 5 | observability_service.py:505 + manager.py:447 |
multiple | Medium | CWE-668 | Attribute rename loop duplicated verbatim 3 times across two files; future security behavior added to one copy will silently diverge |
| 6 | span_attribute_customizer.py:101 |
101 | Low | CWE-95 | Comment "can be enhanced with safe eval" invites eval() injection by future developers |
| 7 | span_attribute_customizer.py:173 |
173–180 | Low | CWE-362 | GlobalContext.state is an unguarded plain dict; non-atomic read-modify-write in multi-thread Gunicorn/Granian modes |
| 8 | span-attribute-customization.md:208 |
208, 258 | Low | CWE-94 | Docs show "{{ tenant_id }}" / "{{ POD_NAME }}" implying Jinja2 support that does not exist; invites SSTI if ever implemented without review |
Remediation suggestions
- Finding 1: Use HMAC-SHA-256 with
settings.auth_encryption_secret, keep ≥32 hex chars. Document it as pseudonymization, not anonymization. - Finding 2: In
resource_pre_fetch, set all three state keys (custom_span_attributes,remove_span_attributes,span_attribute_mapping) explicitly. - Finding 3: Change
condition.split("==")→condition.split("==", 1). Add startup validation against^[a-zA-Z_.]+\s*==\s*"[^"]*"$. - Finding 4: Add Pydantic
field_validatoron attribute dicts limiting values tostr|int|float|booland capping key/count sizes. - Finding 5: Extract to a single
apply_attribute_mapping(attrs, mapping)helper.
Redundant Code
| # | File | Line(s) | Type | Description | Suggestion |
|---|---|---|---|---|---|
| 1 | manager.py:447 + observability_service.py:510 |
multiple | Duplicated logic | Attribute rename dict comprehension copy-pasted verbatim across 3 sites | Extract to apply_attribute_mapping() helper |
| 2 | test_span_attribute_mapping_integration.py:60 |
60–90 | Duplicated test logic | Several integration tests manually replicate the manager.py/observability_service.py mapping loop rather than testing the actual implementation |
Replace with tests that call the real code and assert observable outcomes |
| 3 | test_upstream_session_registry_attribute_mapping |
full function | Dead test | Only asserts that a key exists in a manually-constructed dict — no actual behavior under test | Remove or replace with a meaningful assertion |
| 4 | span_attribute_customizer.py:205 |
205–209 | Empty hook | tool_post_invoke returns immediately with no action; declared comment # Can add attributes based on tool results is stale intent |
Either implement or remove the method and the hook from the hooks list |
Security fixes: - Use HMAC-SHA-256 with auth_encryption_secret for hash transformation - Fix condition.split() to handle multiple == in values safely - Add Pydantic validators for global_attributes type safety - Add thread-safe state access with threading.Lock - Reset all state keys in resource_pre_fetch - Remove unsafe eval comment - Fix misleading Jinja2 documentation examples Code quality improvements: - Extract attribute mapping logic to centralized helper function - Update integration tests to use real implementation - Remove dead test with no behavior validation - Remove empty tool_post_invoke hook All linters passing (ruff, bandit, interrogate, pylint 10.00/10) All tests passing (17,677 passed, 549 skipped) Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
- Add attribute_mapping config field to rename span attribute keys - Apply mapping to both tool spans (ObservabilityService) and plugin spans (PluginManager) - Store mapping in context.global_context.state for runtime access - Add 5 unit tests covering mapping scenarios - Update documentation with mapping examples - Backward compatible: empty/missing mapping = no renaming Closes #4331 Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Security fixes: - Use HMAC-SHA-256 with auth_encryption_secret for hash transformation - Fix condition.split() to handle multiple == in values safely - Add Pydantic validators for global_attributes type safety - Add thread-safe state access with threading.Lock - Reset all state keys in resource_pre_fetch - Remove unsafe eval comment - Fix misleading Jinja2 documentation examples Code quality improvements: - Extract attribute mapping logic to centralized helper function - Update integration tests to use real implementation - Remove dead test with no behavior validation - Remove empty tool_post_invoke hook All linters passing (ruff, bandit, interrogate, pylint 10.00/10) All tests passing (17,677 passed, 549 skipped) Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
994d9c3 to
3d87881
Compare
|
Thanks @Lang-Akshay, addressed the findings. |
- Add attribute_mapping config field to rename span attribute keys - Apply mapping to both tool spans (ObservabilityService) and plugin spans (PluginManager) - Store mapping in context.global_context.state for runtime access - Add 5 unit tests covering mapping scenarios - Update documentation with mapping examples - Backward compatible: empty/missing mapping = no renaming Closes #4331 Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Security fixes: - Use HMAC-SHA-256 with auth_encryption_secret for hash transformation - Fix condition.split() to handle multiple == in values safely - Add Pydantic validators for global_attributes type safety - Add thread-safe state access with threading.Lock - Reset all state keys in resource_pre_fetch - Remove unsafe eval comment - Fix misleading Jinja2 documentation examples Code quality improvements: - Extract attribute mapping logic to centralized helper function - Update integration tests to use real implementation - Remove dead test with no behavior validation - Remove empty tool_post_invoke hook All linters passing (ruff, bandit, interrogate, pylint 10.00/10) All tests passing (17,677 passed, 549 skipped) Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
7e58c6d to
6c92216
Compare
…ry span attributes Implements GitHub issue #4274 - Add customizable span attributes to observability. Changes: - Created SpanAttributeCustomizer plugin with full configuration schema - Added support for global attributes, per-tool overrides, transformations, and conditional attributes - Integrated plugin with ObservabilityService via context parameter passing - Updated start_span(), trace_span(), and trace_tool_invocation() to accept context - Plugin reads custom_span_attributes and remove_span_attributes from GlobalContext.state - Added example configuration to plugins/config.yaml Plugin features: - Global attributes for all spans - Per-tool attribute overrides - Attribute transformations (hash, uppercase, lowercase, truncate) - Conditional attributes based on runtime conditions - Attribute removal for privacy/compliance Integration pattern: - Plugin stores attributes in context.global_context.state during pre-invoke hooks - ObservabilityService reads from context during span creation - Follows established plugin framework patterns Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Adds 28 unit tests covering all plugin functionality with 92% code coverage. Test coverage: - Plugin initialization (basic and empty config) - Global attributes injection - Per-tool attribute overrides - Attribute transformations (hash, uppercase, lowercase, truncate) - Conditional attributes based on tool name - Attribute removal (global and per-tool) - Multiple transformations in sequence - Error handling for unknown operations - All hook types (tool_pre_invoke, tool_post_invoke, resource_pre_fetch, resource_post_fetch) - Configuration schema validation - Complex multi-feature scenarios Coverage: 92% (99 statements, 8 missed) - Missed lines are edge cases in condition evaluation All tests pass successfully. Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Adds 7 integration tests for end-to-end verification of SpanAttributeCustomizer plugin. Integration tests: - Global attributes injected into spans - Transformed attributes in spans (hash, uppercase) - Attribute removal from spans - Tool-specific overrides - Conditional attributes - Backward compatibility (spans without plugin context) - Complex multi-feature scenario Also fixes ruff linting issues: - Added missing 'context' parameter documentation in trace_span() docstring - Added missing 'context' parameter documentation in trace_tool_invocation() docstring Note: Integration tests require database initialization (observability tables) to run. Tests demonstrate correct functionality via captured logs showing proper attribute injection. Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Adds detailed documentation for customizing OpenTelemetry span attributes: New documentation: - docs/docs/manage/span-attribute-customization.md - Complete user guide with: - Quick start guide - Configuration options (global, per-tool, transformations, conditions, removal) - Use cases (cost attribution, compliance, multi-region, service mesh, audit) - Complete configuration example - Verification steps (Jaeger, Prometheus) - Performance and security considerations - Troubleshooting guide Updated documentation: - docs/docs/architecture/otel-span-attributes.md - Added section on customizing span attributes - Setup instructions - Configuration example - Link to detailed plugin README Documentation covers: - 5 core capabilities - 6 real-world use cases - Complete configuration examples - Verification and troubleshooting Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
- Add nosec comment for non-cryptographic random usage in plugin backoff - Add nosec comments for intentional try/except/pass fallback patterns - Replace assert with proper runtime check in upstream session registry - Fix implicit string concatenation in tool plugin binding service - Add pylint disable for too-many-locals in observability service All security scans now pass with 0 issues. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
- Add test for custom span attributes from plugin context - Add test for removing span attributes via plugin context - Covers observability_service.py lines 506-516 These tests verify the SpanAttributeCustomizerPlugin integration with the observability service's start_span method. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Remove trailing whitespace from test functions for code style consistency. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
- Add attribute_mapping config field to rename span attribute keys - Apply mapping to both tool spans (ObservabilityService) and plugin spans (PluginManager) - Store mapping in context.global_context.state for runtime access - Add 5 unit tests covering mapping scenarios - Update documentation with mapping examples - Backward compatible: empty/missing mapping = no renaming Closes #4331 Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Security fixes: - Use HMAC-SHA-256 with auth_encryption_secret for hash transformation - Fix condition.split() to handle multiple == in values safely - Add Pydantic validators for global_attributes type safety - Add thread-safe state access with threading.Lock - Reset all state keys in resource_pre_fetch - Remove unsafe eval comment - Fix misleading Jinja2 documentation examples Code quality improvements: - Extract attribute mapping logic to centralized helper function - Update integration tests to use real implementation - Remove dead test with no behavior validation - Remove empty tool_post_invoke hook All linters passing (ruff, bandit, interrogate, pylint 10.00/10) All tests passing (17,677 passed, 549 skipped) Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
- Add test_executor_observability_span.py with span attribute tests - Add test_executor_otel_mapping.py with OTEL mapping tests - Improves diff-coverage from 22% to 49% - Core attribute mapping functionality now has 100% coverage Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
6c92216 to
60212c3
Compare
Lang-Akshay
left a comment
There was a problem hiding this comment.
Thanks for the addressing previously mentioned issue @vishu-bh
- ✅ SHA-256 → HMAC-SHA-256
- ✅ split("==", 1)
- ✅ global_attributes field_validator
- ✅ apply_attribute_mapping helper extracted
- ✅ plugin disabled by default
- ✅ resource_pre_fetch resets all 3 state keys
Please address this issue
Finding 1: Race Condition in State Access
File: span_attribute_customizer.py:195
Lines: 195–228
Severity: High
CWE: CWE-362 (Concurrent Execution using Shared Resource with Improper Synchronization)
Description
_state_lock guards writes only. observability_service.start_span reads all 3 state keys locklessly. Concurrent resource_pre_fetch (which resets span_attribute_mapping={}) can execute between consecutive reads in start_span, producing a torn read: tool-phase custom_attrs + empty mapping/removal from resource phase.
Remediation
Snapshot all 3 state keys under _state_lock before passing to start_span, or use contextvars.ContextVar per async task.
Example Implementation:
with self._state_lock:
snapshot = {
'custom_attrs': self._custom_attrs.copy(),
'mapping': self._get_attribute_mapping().copy(),
'removal': self._attributes_to_remove.copy()
}
# Pass snapshot to start_spanFinding 2: Insufficient Input Validation
File: config_schema.py:22
Lines: 22, 31
Severity: Medium
CWE: CWE-20 (Improper Input Validation)
Description
ToolOverride.attributes and ConditionalAttribute.add are typed Dict[str, Any] with no validators — allows nested objects, lists, None. OTEL SDK silently rejects non-primitive types. No entry count or value size limits.
Remediation
Add @field_validator to ToolOverride.attributes and ConditionalAttribute.add mirroring validate_global_attributes.
Example Implementation:
@field_validator("attributes")
@classmethod
def validate_attributes(cls, v: Dict[str, Any]) -> Dict[str, Any]:
"""Validate that all attribute values are primitives."""
if not v:
return v
for key, value in v.items():
if not isinstance(value, (str, int, float, bool)):
raise ValueError(
f"Attribute '{key}' has invalid type {type(value).__name__}. "
"Only str, int, float, bool are allowed."
)
return vFinding 3: Inconsistent Attribute Mapping Application
File: span_attribute_customizer.py:219
Lines: 219–228
Severity: Medium
CWE: CWE-840 (Business Logic Errors)
Description
resource_pre_fetch resets span_attribute_mapping={} — attribute renaming never applies to resource spans. README/docs state mapping applies to all spans. Operators configuring renaming for privacy/compliance receive no sanitisation on resource spans — silent security-policy bypass.
Evidence
1. README.md (line ~40) claims:
attribute_mapping:
# Rename plugin span attributes
"plugin.name": "controls.artifact.name"
"plugin.uuid": "controls.artifact.id"Key Points:
- Applies to ALL spans (tools, resources, plugins)
- Original names are replaced with mapped names
2. Code reality (span_attribute_customizer.py:219):
async def resource_pre_fetch(...):
# Reset all state keys to prevent stale tool-invocation state
with self._state_lock:
context.global_context.state["custom_span_attributes"] = custom_attrs
context.global_context.state["remove_span_attributes"] = []
context.global_context.state["span_attribute_mapping"] = {} # ← EMPTY!3. Security Impact:
- Operators configure attribute renaming for compliance (e.g.,
"user.email" → "user.id_hash") - They believe mapping applies to ALL spans per README documentation
- Resource spans retain original attribute names, potentially exposing sensitive data
- Silent failure - no warning that mapping doesn't apply to resources
- This constitutes a security-policy bypass for compliance/privacy requirements
Remediation
Pass self._get_attribute_mapping() through resource_pre_fetch instead of {}, or document intentionally and test it.
Option 1 - Apply mapping to resource spans:
async def resource_pre_fetch(
self, payload: ResourcePreFetchPayload, context: PluginContext
) -> ResourcePreFetchResult:
with self._state_lock:
self._custom_attrs = {}
self._span_attribute_mapping = self._get_attribute_mapping() # Keep mapping
self._attributes_to_remove = set()Option 2 - Document and test the intentional behavior:
- Update README to explicitly state: "Attribute mapping applies only to tool and prompt spans, not resource spans"
- Add test case verifying resource spans use original attribute names
Finding 4: Resource Exhaustion via Unbounded Configuration
File: config_schema.py:57
Lines: 57–70
Severity: Medium
CWE: CWE-400 (Uncontrolled Resource Consumption)
Description
tool_overrides, conditions, transformations, and attribute_mapping have no entry-count limits. Config-write access (admin role) can push thousands of entries, causing O(n²) per-span work — sustained DoS.
Remediation
Add count limits (≤ 200 tool_overrides, ≤ 50 conditions/transformations, ≤ 100 attribute_mapping).
Example Implementation:
@field_validator("tool_overrides")
@classmethod
def validate_tool_overrides_count(cls, v: List[ToolOverride]) -> List[ToolOverride]:
"""Limit tool_overrides to prevent resource exhaustion."""
if len(v) > 200:
raise ValueError(
f"tool_overrides count ({len(v)}) exceeds maximum allowed (200)"
)
return v
@field_validator("conditions")
@classmethod
def validate_conditions_count(cls, v: List[ConditionalAttribute]) -> List[ConditionalAttribute]:
"""Limit conditions to prevent resource exhaustion."""
if len(v) > 50:
raise ValueError(
f"conditions count ({len(v)}) exceeds maximum allowed (50)"
)
return v
@field_validator("transformations")
@classmethod
def validate_transformations_count(cls, v: List[AttributeTransformation]) -> List[AttributeTransformation]:
"""Limit transformations to prevent resource exhaustion."""
if len(v) > 50:
raise ValueError(
f"transformations count ({len(v)}) exceeds maximum allowed (50)"
)
return v
@field_validator("attribute_mapping")
@classmethod
def validate_attribute_mapping_count(cls, v: Dict[str, str]) -> Dict[str, str]:
"""Limit attribute_mapping to prevent resource exhaustion."""
if len(v) > 100:
raise ValueError(
f"attribute_mapping count ({len(v)}) exceeds maximum allowed (100)"
)
return vSigned-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Lang-Akshay
left a comment
There was a problem hiding this comment.
Thanks for the PR @vishu-bh
brian-hussey
left a comment
There was a problem hiding this comment.
Approved based on other reviews from the team.
…ry span attributes (#4331) * feat: Add SpanAttributeCustomizer plugin for customizable OpenTelemetry span attributes Implements GitHub issue #4274 - Add customizable span attributes to observability. Changes: - Created SpanAttributeCustomizer plugin with full configuration schema - Added support for global attributes, per-tool overrides, transformations, and conditional attributes - Integrated plugin with ObservabilityService via context parameter passing - Updated start_span(), trace_span(), and trace_tool_invocation() to accept context - Plugin reads custom_span_attributes and remove_span_attributes from GlobalContext.state - Added example configuration to plugins/config.yaml Plugin features: - Global attributes for all spans - Per-tool attribute overrides - Attribute transformations (hash, uppercase, lowercase, truncate) - Conditional attributes based on runtime conditions - Attribute removal for privacy/compliance Integration pattern: - Plugin stores attributes in context.global_context.state during pre-invoke hooks - ObservabilityService reads from context during span creation - Follows established plugin framework patterns Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: Add comprehensive unit tests for SpanAttributeCustomizer plugin Adds 28 unit tests covering all plugin functionality with 92% code coverage. Test coverage: - Plugin initialization (basic and empty config) - Global attributes injection - Per-tool attribute overrides - Attribute transformations (hash, uppercase, lowercase, truncate) - Conditional attributes based on tool name - Attribute removal (global and per-tool) - Multiple transformations in sequence - Error handling for unknown operations - All hook types (tool_pre_invoke, tool_post_invoke, resource_pre_fetch, resource_post_fetch) - Configuration schema validation - Complex multi-feature scenarios Coverage: 92% (99 statements, 8 missed) - Missed lines are edge cases in condition evaluation All tests pass successfully. Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: Add integration tests and fix docstring linting issues Adds 7 integration tests for end-to-end verification of SpanAttributeCustomizer plugin. Integration tests: - Global attributes injected into spans - Transformed attributes in spans (hash, uppercase) - Attribute removal from spans - Tool-specific overrides - Conditional attributes - Backward compatibility (spans without plugin context) - Complex multi-feature scenario Also fixes ruff linting issues: - Added missing 'context' parameter documentation in trace_span() docstring - Added missing 'context' parameter documentation in trace_tool_invocation() docstring Note: Integration tests require database initialization (observability tables) to run. Tests demonstrate correct functionality via captured logs showing proper attribute injection. Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * docs: Add comprehensive documentation for SpanAttributeCustomizer plugin Adds detailed documentation for customizing OpenTelemetry span attributes: New documentation: - docs/docs/manage/span-attribute-customization.md - Complete user guide with: - Quick start guide - Configuration options (global, per-tool, transformations, conditions, removal) - Use cases (cost attribution, compliance, multi-region, service mesh, audit) - Complete configuration example - Verification steps (Jaeger, Prometheus) - Performance and security considerations - Troubleshooting guide Updated documentation: - docs/docs/architecture/otel-span-attributes.md - Added section on customizing span attributes - Setup instructions - Configuration example - Link to detailed plugin README Documentation covers: - 5 core capabilities - 6 real-world use cases - Complete configuration examples - Verification and troubleshooting Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: resolve bandit and pylint security/quality issues - Add nosec comment for non-cryptographic random usage in plugin backoff - Add nosec comments for intentional try/except/pass fallback patterns - Replace assert with proper runtime check in upstream session registry - Fix implicit string concatenation in tool plugin binding service - Add pylint disable for too-many-locals in observability service All security scans now pass with 0 issues. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: add coverage for plugin context attributes in observability - Add test for custom span attributes from plugin context - Add test for removing span attributes via plugin context - Covers observability_service.py lines 506-516 These tests verify the SpanAttributeCustomizerPlugin integration with the observability service's start_span method. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * style: fix trailing whitespace in observability tests Remove trailing whitespace from test functions for code style consistency. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: add attribute name mapping to SpanAttributeCustomizer plugin - Add attribute_mapping config field to rename span attribute keys - Apply mapping to both tool spans (ObservabilityService) and plugin spans (PluginManager) - Store mapping in context.global_context.state for runtime access - Add 5 unit tests covering mapping scenarios - Update documentation with mapping examples - Backward compatible: empty/missing mapping = no renaming Closes #4331 Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: increasing coverage Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: disabling span plugin by default Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: address PR #4331 security and redundant code issues Security fixes: - Use HMAC-SHA-256 with auth_encryption_secret for hash transformation - Fix condition.split() to handle multiple == in values safely - Add Pydantic validators for global_attributes type safety - Add thread-safe state access with threading.Lock - Reset all state keys in resource_pre_fetch - Remove unsafe eval comment - Fix misleading Jinja2 documentation examples Code quality improvements: - Extract attribute mapping logic to centralized helper function - Update integration tests to use real implementation - Remove dead test with no behavior validation - Remove empty tool_post_invoke hook All linters passing (ruff, bandit, interrogate, pylint 10.00/10) All tests passing (17,677 passed, 549 skipped) Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: review comments addressed Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat:coverage address Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: linter fix Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: removing unwanted tests Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: add comprehensive unit tests for attribute mapping feature - Add test_executor_observability_span.py with span attribute tests - Add test_executor_otel_mapping.py with OTEL mapping tests - Improves diff-coverage from 22% to 49% - Core attribute mapping functionality now has 100% coverage Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: addressing lint failures Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: updated code to address findings Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> --------- Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Merge upstream/main (88 commits) into feat/cpex. Resolved conflicts from 3 PRs that modified the now-removed in-tree plugin framework: - #4292 (runtime plugin management) — refactored Redis/state into CF modules - #4331 (span attribute customizer) — moved attribute mapping to CF - #3152 (identity propagation) — kept UserContext in CF, not CPEX All mcpgateway/plugins/framework/ files remain deleted; functionality is provided by cpex>=0.1.0.dev11 and CF-side modules. New CF-side modules created: - mcpgateway/plugins/_redis.py — Redis provider shim - mcpgateway/plugins/_state.py — per-plugin mode override state - mcpgateway/plugins/utils.py — apply_attribute_mapping utility - mcpgateway/transports/context.py — UserContext model Updated mcpgateway/plugins/__init__.py with runtime management functions (pub/sub invalidation, shared toggle, mode overrides). Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
…ry span attributes (#4331) * feat: Add SpanAttributeCustomizer plugin for customizable OpenTelemetry span attributes Implements GitHub issue #4274 - Add customizable span attributes to observability. Changes: - Created SpanAttributeCustomizer plugin with full configuration schema - Added support for global attributes, per-tool overrides, transformations, and conditional attributes - Integrated plugin with ObservabilityService via context parameter passing - Updated start_span(), trace_span(), and trace_tool_invocation() to accept context - Plugin reads custom_span_attributes and remove_span_attributes from GlobalContext.state - Added example configuration to plugins/config.yaml Plugin features: - Global attributes for all spans - Per-tool attribute overrides - Attribute transformations (hash, uppercase, lowercase, truncate) - Conditional attributes based on runtime conditions - Attribute removal for privacy/compliance Integration pattern: - Plugin stores attributes in context.global_context.state during pre-invoke hooks - ObservabilityService reads from context during span creation - Follows established plugin framework patterns Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: Add comprehensive unit tests for SpanAttributeCustomizer plugin Adds 28 unit tests covering all plugin functionality with 92% code coverage. Test coverage: - Plugin initialization (basic and empty config) - Global attributes injection - Per-tool attribute overrides - Attribute transformations (hash, uppercase, lowercase, truncate) - Conditional attributes based on tool name - Attribute removal (global and per-tool) - Multiple transformations in sequence - Error handling for unknown operations - All hook types (tool_pre_invoke, tool_post_invoke, resource_pre_fetch, resource_post_fetch) - Configuration schema validation - Complex multi-feature scenarios Coverage: 92% (99 statements, 8 missed) - Missed lines are edge cases in condition evaluation All tests pass successfully. Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: Add integration tests and fix docstring linting issues Adds 7 integration tests for end-to-end verification of SpanAttributeCustomizer plugin. Integration tests: - Global attributes injected into spans - Transformed attributes in spans (hash, uppercase) - Attribute removal from spans - Tool-specific overrides - Conditional attributes - Backward compatibility (spans without plugin context) - Complex multi-feature scenario Also fixes ruff linting issues: - Added missing 'context' parameter documentation in trace_span() docstring - Added missing 'context' parameter documentation in trace_tool_invocation() docstring Note: Integration tests require database initialization (observability tables) to run. Tests demonstrate correct functionality via captured logs showing proper attribute injection. Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * docs: Add comprehensive documentation for SpanAttributeCustomizer plugin Adds detailed documentation for customizing OpenTelemetry span attributes: New documentation: - docs/docs/manage/span-attribute-customization.md - Complete user guide with: - Quick start guide - Configuration options (global, per-tool, transformations, conditions, removal) - Use cases (cost attribution, compliance, multi-region, service mesh, audit) - Complete configuration example - Verification steps (Jaeger, Prometheus) - Performance and security considerations - Troubleshooting guide Updated documentation: - docs/docs/architecture/otel-span-attributes.md - Added section on customizing span attributes - Setup instructions - Configuration example - Link to detailed plugin README Documentation covers: - 5 core capabilities - 6 real-world use cases - Complete configuration examples - Verification and troubleshooting Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: resolve bandit and pylint security/quality issues - Add nosec comment for non-cryptographic random usage in plugin backoff - Add nosec comments for intentional try/except/pass fallback patterns - Replace assert with proper runtime check in upstream session registry - Fix implicit string concatenation in tool plugin binding service - Add pylint disable for too-many-locals in observability service All security scans now pass with 0 issues. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: add coverage for plugin context attributes in observability - Add test for custom span attributes from plugin context - Add test for removing span attributes via plugin context - Covers observability_service.py lines 506-516 These tests verify the SpanAttributeCustomizerPlugin integration with the observability service's start_span method. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * style: fix trailing whitespace in observability tests Remove trailing whitespace from test functions for code style consistency. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: add attribute name mapping to SpanAttributeCustomizer plugin - Add attribute_mapping config field to rename span attribute keys - Apply mapping to both tool spans (ObservabilityService) and plugin spans (PluginManager) - Store mapping in context.global_context.state for runtime access - Add 5 unit tests covering mapping scenarios - Update documentation with mapping examples - Backward compatible: empty/missing mapping = no renaming Closes #4331 Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: increasing coverage Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: disabling span plugin by default Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: address PR #4331 security and redundant code issues Security fixes: - Use HMAC-SHA-256 with auth_encryption_secret for hash transformation - Fix condition.split() to handle multiple == in values safely - Add Pydantic validators for global_attributes type safety - Add thread-safe state access with threading.Lock - Reset all state keys in resource_pre_fetch - Remove unsafe eval comment - Fix misleading Jinja2 documentation examples Code quality improvements: - Extract attribute mapping logic to centralized helper function - Update integration tests to use real implementation - Remove dead test with no behavior validation - Remove empty tool_post_invoke hook All linters passing (ruff, bandit, interrogate, pylint 10.00/10) All tests passing (17,677 passed, 549 skipped) Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: review comments addressed Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat:coverage address Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: linter fix Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: removing unwanted tests Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: add comprehensive unit tests for attribute mapping feature - Add test_executor_observability_span.py with span attribute tests - Add test_executor_otel_mapping.py with OTEL mapping tests - Improves diff-coverage from 22% to 49% - Core attribute mapping functionality now has 100% coverage Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: addressing lint failures Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: updated code to address findings Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> --------- Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com>
…ry span attributes (#4331) * feat: Add SpanAttributeCustomizer plugin for customizable OpenTelemetry span attributes Implements GitHub issue #4274 - Add customizable span attributes to observability. Changes: - Created SpanAttributeCustomizer plugin with full configuration schema - Added support for global attributes, per-tool overrides, transformations, and conditional attributes - Integrated plugin with ObservabilityService via context parameter passing - Updated start_span(), trace_span(), and trace_tool_invocation() to accept context - Plugin reads custom_span_attributes and remove_span_attributes from GlobalContext.state - Added example configuration to plugins/config.yaml Plugin features: - Global attributes for all spans - Per-tool attribute overrides - Attribute transformations (hash, uppercase, lowercase, truncate) - Conditional attributes based on runtime conditions - Attribute removal for privacy/compliance Integration pattern: - Plugin stores attributes in context.global_context.state during pre-invoke hooks - ObservabilityService reads from context during span creation - Follows established plugin framework patterns Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: Add comprehensive unit tests for SpanAttributeCustomizer plugin Adds 28 unit tests covering all plugin functionality with 92% code coverage. Test coverage: - Plugin initialization (basic and empty config) - Global attributes injection - Per-tool attribute overrides - Attribute transformations (hash, uppercase, lowercase, truncate) - Conditional attributes based on tool name - Attribute removal (global and per-tool) - Multiple transformations in sequence - Error handling for unknown operations - All hook types (tool_pre_invoke, tool_post_invoke, resource_pre_fetch, resource_post_fetch) - Configuration schema validation - Complex multi-feature scenarios Coverage: 92% (99 statements, 8 missed) - Missed lines are edge cases in condition evaluation All tests pass successfully. Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: Add integration tests and fix docstring linting issues Adds 7 integration tests for end-to-end verification of SpanAttributeCustomizer plugin. Integration tests: - Global attributes injected into spans - Transformed attributes in spans (hash, uppercase) - Attribute removal from spans - Tool-specific overrides - Conditional attributes - Backward compatibility (spans without plugin context) - Complex multi-feature scenario Also fixes ruff linting issues: - Added missing 'context' parameter documentation in trace_span() docstring - Added missing 'context' parameter documentation in trace_tool_invocation() docstring Note: Integration tests require database initialization (observability tables) to run. Tests demonstrate correct functionality via captured logs showing proper attribute injection. Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * docs: Add comprehensive documentation for SpanAttributeCustomizer plugin Adds detailed documentation for customizing OpenTelemetry span attributes: New documentation: - docs/docs/manage/span-attribute-customization.md - Complete user guide with: - Quick start guide - Configuration options (global, per-tool, transformations, conditions, removal) - Use cases (cost attribution, compliance, multi-region, service mesh, audit) - Complete configuration example - Verification steps (Jaeger, Prometheus) - Performance and security considerations - Troubleshooting guide Updated documentation: - docs/docs/architecture/otel-span-attributes.md - Added section on customizing span attributes - Setup instructions - Configuration example - Link to detailed plugin README Documentation covers: - 5 core capabilities - 6 real-world use cases - Complete configuration examples - Verification and troubleshooting Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: resolve bandit and pylint security/quality issues - Add nosec comment for non-cryptographic random usage in plugin backoff - Add nosec comments for intentional try/except/pass fallback patterns - Replace assert with proper runtime check in upstream session registry - Fix implicit string concatenation in tool plugin binding service - Add pylint disable for too-many-locals in observability service All security scans now pass with 0 issues. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: add coverage for plugin context attributes in observability - Add test for custom span attributes from plugin context - Add test for removing span attributes via plugin context - Covers observability_service.py lines 506-516 These tests verify the SpanAttributeCustomizerPlugin integration with the observability service's start_span method. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * style: fix trailing whitespace in observability tests Remove trailing whitespace from test functions for code style consistency. Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: add attribute name mapping to SpanAttributeCustomizer plugin - Add attribute_mapping config field to rename span attribute keys - Apply mapping to both tool spans (ObservabilityService) and plugin spans (PluginManager) - Store mapping in context.global_context.state for runtime access - Add 5 unit tests covering mapping scenarios - Update documentation with mapping examples - Backward compatible: empty/missing mapping = no renaming Closes #4331 Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: increasing coverage Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: disabling span plugin by default Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: address PR #4331 security and redundant code issues Security fixes: - Use HMAC-SHA-256 with auth_encryption_secret for hash transformation - Fix condition.split() to handle multiple == in values safely - Add Pydantic validators for global_attributes type safety - Add thread-safe state access with threading.Lock - Reset all state keys in resource_pre_fetch - Remove unsafe eval comment - Fix misleading Jinja2 documentation examples Code quality improvements: - Extract attribute mapping logic to centralized helper function - Update integration tests to use real implementation - Remove dead test with no behavior validation - Remove empty tool_post_invoke hook All linters passing (ruff, bandit, interrogate, pylint 10.00/10) All tests passing (17,677 passed, 549 skipped) Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: review comments addressed Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat:coverage address Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: linter fix Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * fix: removing unwanted tests Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * test: add comprehensive unit tests for attribute mapping feature - Add test_executor_observability_span.py with span attribute tests - Add test_executor_otel_mapping.py with OTEL mapping tests - Improves diff-coverage from 22% to 49% - Core attribute mapping functionality now has 100% coverage Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: addressing lint failures Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> * feat: updated code to address findings Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com> --------- Signed-off-by: Bob Shell <bob@contextforge.dev> Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Summary
Implements GitHub issue #4274 - Add customizable OpenTelemetry span attributes to observability system.
Changes
Plugin Implementation
SpanAttributeCustomizerPluginwith comprehensive feature setIntegration
ObservabilityServiceto accept plugin contextstart_span(),trace_span(), andtrace_tool_invocation()GlobalContext.stateTesting
Configuration
plugins/config.yamlFiles Changed
plugins/span_attribute_customizer/(new plugin)mcpgateway/services/observability_service.py(integration)plugins/config.yaml(configuration example)tests/unit/mcpgateway/plugins/span_attribute_customizer/(unit tests)tests/integration/plugins/(integration tests)Testing Plan
Closes
Closes #4274