Ben/category enrichment #32
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,39 @@ | ||
| import warnings | ||
| import urllib3 | ||
| from langfuse import Langfuse | ||
| from opentelemetry import trace as otel_trace_api | ||
| from agent.config import settings | ||
|
|
||
| # Suppress unverified HTTPS warnings for dev internal endpoints | ||
| urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) | ||
| warnings.filterwarnings("ignore", category=urllib3.exceptions.InsecureRequestWarning) | ||
|
Comment on lines
+7
to
+9
There was a problem hiding this comment. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Global warning suppression hides TLS problems for the whole process.
Restrict the suppression to the development case, for example by gating it on a setting, and keep verification enabled elsewhere. 🔒 Proposed fix-# Suppress unverified HTTPS warnings for dev internal endpoints
-urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)
-warnings.filterwarnings("ignore", category=urllib3.exceptions.InsecureRequestWarning)
+# Suppress unverified HTTPS warnings for dev internal endpoints only.
+if getattr(settings, "ALLOW_INSECURE_INTERNAL_TLS", False):
+ urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)
+ warnings.filterwarnings(
+ "ignore", category=urllib3.exceptions.InsecureRequestWarning
+ )🤖 Prompt for AI Agents |
||
|
|
||
| langfuse_client = Langfuse( | ||
| public_key=settings.LANGFUSE_PUBLIC_KEY, | ||
| secret_key=settings.LANGFUSE_SECRET_KEY, | ||
| host=settings.LANGFUSE_BASE_URL, | ||
| ) | ||
|
|
||
| # Patch update_current_span and get_current_trace_id to safely no-op when running without an active OpenTelemetry span context | ||
| _orig_update_current_span = langfuse_client.update_current_span | ||
| _orig_get_current_trace_id = langfuse_client.get_current_trace_id | ||
|
|
||
|
|
||
| def _safe_update_current_span(*args, **kwargs): | ||
| current_span = otel_trace_api.get_current_span() | ||
| if current_span is otel_trace_api.INVALID_SPAN: | ||
| return | ||
| return _orig_update_current_span(*args, **kwargs) | ||
|
|
||
|
|
||
| def _safe_get_current_trace_id(*args, **kwargs): | ||
| current_span = otel_trace_api.get_current_span() | ||
| if current_span is otel_trace_api.INVALID_SPAN: | ||
| return None | ||
| return _orig_get_current_trace_id(*args, **kwargs) | ||
|
Comment on lines
+22
to
+33
There was a problem hiding this comment. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🌐 Web query:
💡 Result: In OpenTelemetry Python, get_current_span is used to retrieve the span currently stored in the execution context [1][2]. If no span is present in the context, it returns an INVALID_SPAN object [3][1]. INVALID_SPAN is a specific instance of a NonRecordingSpan [3][4]. A NonRecordingSpan is a "no-op" span implementation used when no full tracing SDK or instrumentation is active [3][5][6]. Its primary purpose is to allow applications to continue using the OpenTelemetry API (e.g., propagating trace contexts) without crashing or requiring a fully functional tracing backend [3][5]. Key points regarding these components: 1. INVALID_SPAN: This is the default returned by get_current_span when no active span is found [1][2]. It contains an "invalid" SpanContext [4]. 2. NonRecordingSpan: This class implements the Span interface but performs no actual recording of events, attributes, or status updates [3][6]. Its method is_recording consistently returns False [5][6]. 3. SpanContext and is_valid: The SpanContext associated with a span has a property called is_valid [3][6]. This property indicates whether the context contains valid trace and span identifiers [3][7]. In the case of INVALID_SPAN, the associated SpanContext has is_valid set to False (or its equivalent state indicating an invalid/empty context) [5]. You can check the validity of a span's context by accessing span.get_span_context.is_valid [3][6]. If you find that get_current_span is returning an INVALID_SPAN unexpectedly, it often means that no span has been activated in the current execution context [8]. This typically occurs if you have not started a span using a tracer (e.g., using tracer.start_as_current_span or the context manager) or if the scope where the span was activated has already closed [8]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Repository files matching langfuse_client.py:"
fd -a 'langfuse_client.py' . || true
echo
echo "Relevant file excerpt:"
if [ -f agent/src/agent/langfuse_client.py ]; then
cat -n agent/src/agent/langfuse_client.py | sed -n '1,120p'
else
echo "file not found"
fi
echo
echo "OpenTelemetry API/imports and span context usage:"
rg -n "INVALID_SPAN|get_current_span|NonRecordingSpan|is_valid|get_span_context" -S . --glob '!node_modules' --glob '!dist' --glob '!build' || true
echo
echo "Check installed opentelemetry API availability and behavior without importing repo code:"
python3 - <<'PY'
import importlib.util
for mod in ['opentelemetry.trace','opentelemetry.trace.span']:
print(f'{mod}: {importlib.util.find_spec(mod) is not None}')
if importlib.util.find_spec('opentelemetry.trace') is not None:
import opentelemetry.trace as trace_api
print('INVALID_SPAN id:', id(trace_api.INVALID_SPAN))
print('INVALID_SPAN __class__:', trace_api.INVALID_SPAN.__class__)
print('INVALID_SPAN is_valid:', trace_api.INVALID_SPAN.get_span_context().is_valid if hasattr(trace_api.INVALID_SPAN,'get_span_context') else 'no get_span_context')
print('get_current_span():', trace_api.get_current_span())
print('current is same INSTANCE:', trace_api.get_current_span() is trace_api.INVALID_SPAN)
print('current is_valid:', trace_api.get_current_span().get_span_context().is_valid if hasattr(trace_api.get_current_span(),'get_span_context') else 'no get_span_context')
PYRepository: StavPonte11/text2sql-onboarding Length of output: 2762 🌐 Web query:
💡 Result: In OpenTelemetry Python, the behavior of span-related objects and the retrieval of the current span are designed to ensure applications do not crash when tracing is not active or when a span is explicitly configured not to record data [1][2]. get_current_span and InvalidSpan The function opentelemetry.trace.get_current_span is used to retrieve the currently active span from the context [3][4]. If no span is active in the current context, it returns an INVALID_SPAN object [3][4]. INVALID_SPAN is a singleton that implements the Span interface as a no-op; calling methods on it (such as set_attribute or end) does nothing, ensuring that your application code remains safe even if the tracing infrastructure is not initialized [5][2]. NonRecordingSpan A NonRecordingSpan is a specific type of span that exists to facilitate context propagation even when a span is not intended to be recorded (e.g., due to sampling decisions) [5][1]. - Functionality: Like INVALID_SPAN, a NonRecordingSpan is essentially a no-op span [5]. Its primary purpose is to hold SpanContext information (such as TraceId and SpanId) so that trace context can be propagated to downstream services without incurring the overhead of recording span data [5][6]. - Propagation: When trace context is extracted (e.g., from incoming HTTP headers), a NonRecordingSpan may be created if the incoming context indicates that the trace should not be sampled (e.g., the sampled flag is set to 0 in the W3C traceparent header) [7][6][8]. - Confusion in Behavior: Users often encounter confusion when they see spans are not recorded when a traceparent header is present [7][8]. This is typically due to the default ParentBased sampler, which respects the sampling decision propagated from the upstream service [7]. If the upstream service decided not to sample the request, the Python application will create a NonRecordingSpan for the child, causing it to appear as if tracing is broken [7][8]. Summary of Key Differences - INVALID_SPAN: Returned by get_current_span when absolutely no span is associated with the current context [3][4]. It is the "default" fallback [2]. - NonRecordingSpan: Returned when a context exists and needs to be propagated, but the sampling configuration (or inherited sampling decision) dictates that no span data should be recorded [5][7][6]. If you are experiencing unexpected non-recording behavior, verify your sampling configuration (e.g., OTEL_TRACES_SAMPLER environment variable) and ensure you are not explicitly passing or inheriting a non-sampled context [7][8]. Citations:
🌐 Web query:
💡 Result: In the OpenTelemetry Python API, NonRecordingSpan is a concrete implementation of the Span interface used when no functional tracing SDK is configured or active [1][2]. Its primary purpose is to allow applications to use the OpenTelemetry API without side effects, while still supporting context propagation [1][3]. NonRecordingSpan and its methods behave as follows: is_recording: This method always returns False for a NonRecordingSpan [2][4]. This signals that the span is not capturing any data such as attributes, events, or status updates [3]. All operations that would typically record data (e.g., set_attribute, add_event) are no-op in this class [2][4]. get_span_context: This method returns the SpanContext associated with the NonRecordingSpan [2][4]. Even when a span is non-recording, it may hold a valid SpanContext to ensure that trace and span identifiers can be propagated to downstream services or child spans [3][5]. SpanContext.is_valid: The is_valid property on a SpanContext indicates whether the context contains a properly formatted, non-zero trace ID and span ID [6][7]. A SpanContext is considered valid if the trace ID is greater than the INVALID_TRACE_ID and the span ID is greater than the INVALID_SPAN_ID [7]. In summary, a NonRecordingSpan is a lightweight, no-op span that maintains a SpanContext to facilitate trace propagation, but does not perform any data recording [3][4]. Its is_recording method consistently returns False [2][4], while the validity of its context is determined by the internal state of the attached SpanContext [7]. Citations:
Test span recording and context validity, not object identity.
🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| langfuse_client.update_current_span = _safe_update_current_span | ||
| langfuse_client.get_current_trace_id = _safe_get_current_trace_id | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,3 +69,4 @@ def get_llm( | |
| temperature=temperature, | ||
| timeout=300.0, | ||
| ) | ||
|
|
||
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.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Remove the hard-coded API key from source.
Line 29 commits a live-looking
mcp_...bearer token as the default value ofJEEN_API_KEY. The value is now in Git history and is readable by anyone with repository access. The same pattern exists at line 42 forJEEN_METADATA_MCP_KEY. Restore empty defaults and supply the keys through the environment. Rotate both keys, because they must be treated as compromised.The inline comment "If empty, agent gracefully skips fetching" also no longer matches a non-empty default.
🔒 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents