-
Notifications
You must be signed in to change notification settings - Fork 804
feat(langchain): allow configuration of metadata key prefix #3367
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdded a configurable metadata key prefix for per-metadata span attributes in the LangChain instrumentation. Introduced Config.metadata_key_prefix with a default, exposed a new optional constructor parameter to override it, and updated the callback handler to use the configurable prefix when setting span attributes. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to 5bcaf72 in 2 minutes and 36 seconds. Click for details.
- Reviewed
58
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
2
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py:40
- Draft comment:
Missing import for 'Optional' from the typing module. Since you're using 'Optional[str]' in the function signature, addfrom typing import Optional
at the top. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py:56
- Draft comment:
Consider avoiding mutation of the global Config for 'metadata_key_prefix'. Storing this configuration as an instance attribute could improve isolation when multiple instrumentors are used concurrently. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
Workflow ID: wflow_J6RF7kgIJ4ANhUUO
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
_set_span_attribute( | ||
span, | ||
f"{SpanAttributes.TRACELOOP_ASSOCIATION_PROPERTIES}.{key}", | ||
f"{Config.metadata_key_prefix}.{key}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'Config' object is used for constructing the metadata attribute key, but it is not imported in this file. Please add from opentelemetry.instrumentation.langchain.config import Config
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py (1)
289-295
: Fix undefined name: Config.Ruff/Flake8 report F821 at Line 293. Add the missing import.
Apply this import near the other langchain instrumentation imports:
+from opentelemetry.instrumentation.langchain.config import Config
🧹 Nitpick comments (2)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py (1)
289-295
: Normalize custom prefixes to avoid double dots.If a caller supplies a trailing dot (e.g., "langfuse.metadata."), resulting keys become "..key". Strip trailing dots once and reuse.
Apply:
- if metadata is not None: - for key, value in sanitized_metadata.items(): - _set_span_attribute( - span, - f"{Config.metadata_key_prefix}.{key}", - value, - ) + if metadata is not None: + prefix = Config.metadata_key_prefix.rstrip(".") + for key, value in sanitized_metadata.items(): + _set_span_attribute( + span, + f"{prefix}.{key}", + value, + )packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py (1)
56-57
: Use explicit None check and normalize the prefix.Truthiness skips valid empty strings; prefer
is not None
. Also strip trailing dots once.Apply:
- if metadata_key_prefix: - Config.metadata_key_prefix = metadata_key_prefix + if metadata_key_prefix is not None: + Config.metadata_key_prefix = metadata_key_prefix.rstrip(".")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py
(1 hunks)packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py
(1 hunks)packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/config.py
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py
: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/config.py
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py
🧬 Code graph analysis (3)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/config.py (1)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
SpanAttributes
(64-261)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/config.py (1)
Config
(6-10)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/config.py (1)
Config
(6-10)
🪛 Ruff (0.12.2)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py
293-293: Undefined name Config
(F821)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py
40-40: Undefined name Optional
(F821)
🪛 Flake8 (7.2.0)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py
[error] 293-293: undefined name 'Config'
(F821)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py
[error] 40-40: undefined name 'Optional'
(F821)
🔇 Additional comments (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/config.py (1)
4-4
: Configurable prefix default looks good.Importing SpanAttributes and defaulting metadata_key_prefix to TRACELOOP_ASSOCIATION_PROPERTIES matches current behavior while enabling overrides.
Also applies to: 10-10
metadata_key_prefix: Optional[str] = None | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import Optional to satisfy type hints.
Optional
is used in the signature but not imported; Flake8 F821.
Add to the typing import:
-from typing import Collection
+from typing import Collection, Optional
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
metadata_key_prefix: Optional[str] = None | |
): | |
from typing import Collection, Optional |
🧰 Tools
🪛 Ruff (0.12.2)
40-40: Undefined name Optional
(F821)
🪛 Flake8 (7.2.0)
[error] 40-40: undefined name 'Optional'
(F821)
🤖 Prompt for AI Agents
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py
around lines 40-41: the function signature uses Optional in the type hints but
Optional is not imported, causing Flake8 F821; add Optional to the existing
typing import (e.g., from typing import Any, Dict, Iterable, Optional) at the
top of the file so the type hint is resolved.
exception_logger=None, | ||
disable_trace_context_propagation=False, | ||
use_legacy_attributes: bool = True, | ||
metadata_key_prefix: Optional[str] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should default to the way it works tdoay
Allowing to configure the key prefix for metadata enables use on other backends.
As an example for Langfuse v3 otel backend, you can use prefix
langfuse.metadata
to make user.id and session.id available.feat(instrumentation): ...
orfix(instrumentation): ...
.Important
Add configurable metadata key prefix to Langchain instrumentation for better backend integration.
metadata_key_prefix
parameter toLangchainInstrumentor.__init__()
in__init__.py
to allow customization of metadata key prefix.SpanAttributes.TRACELOOP_ASSOCIATION_PROPERTIES
if not specified.metadata_key_prefix
attribute toConfig
class inconfig.py
._create_span()
incallback_handler.py
to useConfig.metadata_key_prefix
for metadata keys.This description was created by
for 5bcaf72. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit