-
Notifications
You must be signed in to change notification settings - Fork 777
feat(semantic-conventions-ai): move custom histogram to semantic-conventions-ai #3181
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?
Changes from all commits
943b7f2
6c4a316
6e0755d
cfa489e
15da3e1
8c93f1e
3e30544
4d1e090
adaa4e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -300,3 +300,192 @@ class TraceloopSpanKindValues(Enum): | |||||||||||||||||||||||||||||||||||||||||||||||
AGENT = "agent" | ||||||||||||||||||||||||||||||||||||||||||||||||
TOOL = "tool" | ||||||||||||||||||||||||||||||||||||||||||||||||
UNKNOWN = "unknown" | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
class MetricBuckets: | ||||||||||||||||||||||||||||||||||||||||||||||||
"""Predefined histogram bucket boundaries for GenAI metrics.""" | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
LLM_TOKEN_USAGE = [ | ||||||||||||||||||||||||||||||||||||||||||||||||
1, | ||||||||||||||||||||||||||||||||||||||||||||||||
4, | ||||||||||||||||||||||||||||||||||||||||||||||||
16, | ||||||||||||||||||||||||||||||||||||||||||||||||
64, | ||||||||||||||||||||||||||||||||||||||||||||||||
256, | ||||||||||||||||||||||||||||||||||||||||||||||||
1024, | ||||||||||||||||||||||||||||||||||||||||||||||||
4096, | ||||||||||||||||||||||||||||||||||||||||||||||||
16384, | ||||||||||||||||||||||||||||||||||||||||||||||||
65536, | ||||||||||||||||||||||||||||||||||||||||||||||||
262144, | ||||||||||||||||||||||||||||||||||||||||||||||||
1048576, | ||||||||||||||||||||||||||||||||||||||||||||||||
4194304, | ||||||||||||||||||||||||||||||||||||||||||||||||
16777216, | ||||||||||||||||||||||||||||||||||||||||||||||||
67108864, | ||||||||||||||||||||||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
PINECONE_DB_QUERY_DURATION = [ | ||||||||||||||||||||||||||||||||||||||||||||||||
0.01, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.02, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.04, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.08, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.16, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.32, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.64, | ||||||||||||||||||||||||||||||||||||||||||||||||
1.28, | ||||||||||||||||||||||||||||||||||||||||||||||||
2.56, | ||||||||||||||||||||||||||||||||||||||||||||||||
5.12, | ||||||||||||||||||||||||||||||||||||||||||||||||
10.24, | ||||||||||||||||||||||||||||||||||||||||||||||||
20.48, | ||||||||||||||||||||||||||||||||||||||||||||||||
40.96, | ||||||||||||||||||||||||||||||||||||||||||||||||
81.92, | ||||||||||||||||||||||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
PINECONE_DB_QUERY_SCORES = [ | ||||||||||||||||||||||||||||||||||||||||||||||||
-1, | ||||||||||||||||||||||||||||||||||||||||||||||||
-0.875, | ||||||||||||||||||||||||||||||||||||||||||||||||
-0.75, | ||||||||||||||||||||||||||||||||||||||||||||||||
-0.625, | ||||||||||||||||||||||||||||||||||||||||||||||||
-0.5, | ||||||||||||||||||||||||||||||||||||||||||||||||
-0.375, | ||||||||||||||||||||||||||||||||||||||||||||||||
-0.25, | ||||||||||||||||||||||||||||||||||||||||||||||||
-0.125, | ||||||||||||||||||||||||||||||||||||||||||||||||
0, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.125, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.25, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.375, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.5, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.625, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.75, | ||||||||||||||||||||||||||||||||||||||||||||||||
0.875, | ||||||||||||||||||||||||||||||||||||||||||||||||
1, | ||||||||||||||||||||||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
class MetricViewsBuilder: | ||||||||||||||||||||||||||||||||||||||||||||||||
"""Builder for OpenTelemetry metric views with predefined buckets.""" | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||
def get_llm_token_usage_view(): | ||||||||||||||||||||||||||||||||||||||||||||||||
from opentelemetry.sdk.metrics.view import View, ExplicitBucketHistogramAggregation | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
return View( | ||||||||||||||||||||||||||||||||||||||||||||||||
instrument_name=Meters.LLM_TOKEN_USAGE, | ||||||||||||||||||||||||||||||||||||||||||||||||
aggregation=ExplicitBucketHistogramAggregation( | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricBuckets.LLM_TOKEN_USAGE | ||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||
def get_pinecone_query_duration_view(): | ||||||||||||||||||||||||||||||||||||||||||||||||
from opentelemetry.sdk.metrics.view import View, ExplicitBucketHistogramAggregation | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
return View( | ||||||||||||||||||||||||||||||||||||||||||||||||
instrument_name=Meters.PINECONE_DB_QUERY_DURATION, | ||||||||||||||||||||||||||||||||||||||||||||||||
aggregation=ExplicitBucketHistogramAggregation( | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricBuckets.PINECONE_DB_QUERY_DURATION | ||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||
def get_pinecone_query_scores_view(): | ||||||||||||||||||||||||||||||||||||||||||||||||
from opentelemetry.sdk.metrics.view import View, ExplicitBucketHistogramAggregation | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
return View( | ||||||||||||||||||||||||||||||||||||||||||||||||
instrument_name=Meters.PINECONE_DB_QUERY_SCORES, | ||||||||||||||||||||||||||||||||||||||||||||||||
aggregation=ExplicitBucketHistogramAggregation( | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricBuckets.PINECONE_DB_QUERY_SCORES | ||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||
def get_llm_operation_duration_view(): | ||||||||||||||||||||||||||||||||||||||||||||||||
from opentelemetry.sdk.metrics.view import View | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
return View( | ||||||||||||||||||||||||||||||||||||||||||||||||
instrument_name=Meters.LLM_OPERATION_DURATION, | ||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||
def get_db_query_duration_view(): | ||||||||||||||||||||||||||||||||||||||||||||||||
from opentelemetry.sdk.metrics.view import View, ExplicitBucketHistogramAggregation | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
return View( | ||||||||||||||||||||||||||||||||||||||||||||||||
instrument_name=Meters.DB_QUERY_DURATION, | ||||||||||||||||||||||||||||||||||||||||||||||||
aggregation=ExplicitBucketHistogramAggregation( | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricBuckets.PINECONE_DB_QUERY_DURATION | ||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+413
to
+416
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Reusing Pinecone duration buckets for generic DB queries may not be appropriate. The Consider adding generic database duration buckets: class MetricBuckets:
# ... existing buckets ...
+
+ DB_QUERY_DURATION = [
+ 0.001, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5,
+ 0.75, 1.0, 2.5, 5.0, 7.5, 10.0, 30.0, 60.0
+ ] Then update the view: aggregation=ExplicitBucketHistogramAggregation(
- MetricBuckets.PINECONE_DB_QUERY_DURATION
+ MetricBuckets.DB_QUERY_DURATION
), 📝 Committable suggestion
Suggested change
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||
def get_db_search_distance_view(): | ||||||||||||||||||||||||||||||||||||||||||||||||
from opentelemetry.sdk.metrics.view import View, ExplicitBucketHistogramAggregation | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
return View( | ||||||||||||||||||||||||||||||||||||||||||||||||
instrument_name=Meters.DB_SEARCH_DISTANCE, | ||||||||||||||||||||||||||||||||||||||||||||||||
aggregation=ExplicitBucketHistogramAggregation( | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricBuckets.PINECONE_DB_QUERY_SCORES | ||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+424
to
+427
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Reusing score buckets for distance metrics may be semantically incorrect. Distance metrics often have different ranges and semantics than similarity scores. For example, Euclidean distance is typically non-negative and unbounded, while cosine similarity is bounded [-1, 1]. Consider defining separate buckets for distance metrics: class MetricBuckets:
# ... existing buckets ...
+
+ DB_SEARCH_DISTANCE = [
+ 0, 0.1, 0.25, 0.5, 0.75, 1.0, 1.5, 2.0, 3.0, 5.0, 10.0, 25.0, 50.0, 100.0
+ ] 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||
def get_all_genai_views(): | ||||||||||||||||||||||||||||||||||||||||||||||||
return [ | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricViewsBuilder.get_llm_token_usage_view(), | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricViewsBuilder.get_pinecone_query_duration_view(), | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricViewsBuilder.get_pinecone_query_scores_view(), | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricViewsBuilder.get_llm_operation_duration_view(), | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricViewsBuilder.get_db_query_duration_view(), | ||||||||||||||||||||||||||||||||||||||||||||||||
MetricViewsBuilder.get_db_search_distance_view(), | ||||||||||||||||||||||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
def apply_genai_bucket_configuration(): | ||||||||||||||||||||||||||||||||||||||||||||||||
"""Apply GenAI bucket configuration to the global MeterProvider.""" | ||||||||||||||||||||||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||||||||||||||||||||||
from opentelemetry import metrics | ||||||||||||||||||||||||||||||||||||||||||||||||
from opentelemetry.sdk.metrics import MeterProvider | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
current_provider = metrics.get_meter_provider() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
if hasattr(current_provider, '_genai_buckets_applied'): | ||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+449
to
+450
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thread safety concern with provider modification. The flag Add thread synchronization: +import threading
+
+_config_lock = threading.Lock()
+
def apply_genai_bucket_configuration():
"""Apply GenAI bucket configuration to the global MeterProvider."""
+ with _config_lock:
try:
# ... rest of the function 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
genai_views = MetricViewsBuilder.get_all_genai_views() | ||||||||||||||||||||||||||||||||||||||||||||||||
provider_type = str(type(current_provider)) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
if ('NoOp' in provider_type or 'Proxy' in provider_type or | ||||||||||||||||||||||||||||||||||||||||||||||||
not hasattr(current_provider, '__module__') or | ||||||||||||||||||||||||||||||||||||||||||||||||
current_provider.__class__.__name__ == 'ProxyMeterProvider'): | ||||||||||||||||||||||||||||||||||||||||||||||||
new_provider = MeterProvider(views=genai_views) | ||||||||||||||||||||||||||||||||||||||||||||||||
new_provider._genai_buckets_applied = True | ||||||||||||||||||||||||||||||||||||||||||||||||
metrics.set_meter_provider(new_provider) | ||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+455
to
+460
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Provider detection logic may be fragile. The string-based detection using Use more robust type checking: - if ('NoOp' in provider_type or 'Proxy' in provider_type or
- not hasattr(current_provider, '__module__') or
- current_provider.__class__.__name__ == 'ProxyMeterProvider'):
+ from opentelemetry.metrics import NoOpMeterProvider
+ from opentelemetry.metrics._internal import ProxyMeterProvider
+
+ if (isinstance(current_provider, (NoOpMeterProvider, ProxyMeterProvider)) or
+ not hasattr(current_provider, '_all_metric_readers')): 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||
existing_readers = [] | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
if hasattr(current_provider, '_all_metric_readers'): | ||||||||||||||||||||||||||||||||||||||||||||||||
old_readers = list(getattr(current_provider, '_all_metric_readers', [])) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
for old_reader in old_readers: | ||||||||||||||||||||||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||||||||||||||||||||||
if hasattr(old_reader, '_exporter') and hasattr(old_reader, '_export_interval_millis'): | ||||||||||||||||||||||||||||||||||||||||||||||||
from opentelemetry.sdk.metrics.export import PeriodicExportingMetricReader | ||||||||||||||||||||||||||||||||||||||||||||||||
new_reader = PeriodicExportingMetricReader( | ||||||||||||||||||||||||||||||||||||||||||||||||
exporter=old_reader._exporter, | ||||||||||||||||||||||||||||||||||||||||||||||||
export_interval_millis=old_reader._export_interval_millis | ||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||
existing_readers.append(new_reader) | ||||||||||||||||||||||||||||||||||||||||||||||||
except Exception: | ||||||||||||||||||||||||||||||||||||||||||||||||
pass | ||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+477
to
+478
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Silent exception handling may hide important errors. The bare Add proper error handling and logging: +import logging
+
+logger = logging.getLogger(__name__)
+
except Exception as e:
- pass
+ logger.debug(f"Failed to recreate metric reader: {e}")
+ continue Apply similar changes to other exception blocks.
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||||||||||||||||||||||
new_provider = MeterProvider( | ||||||||||||||||||||||||||||||||||||||||||||||||
views=genai_views, | ||||||||||||||||||||||||||||||||||||||||||||||||
metric_readers=existing_readers | ||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||
new_provider._genai_buckets_applied = True | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
import opentelemetry.metrics._internal as metrics_internal | ||||||||||||||||||||||||||||||||||||||||||||||||
metrics_internal._METER_PROVIDER = new_provider | ||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+486
to
+487
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Direct manipulation of internal OpenTelemetry state is risky. Directly setting Use the official API instead: - import opentelemetry.metrics._internal as metrics_internal
- metrics_internal._METER_PROVIDER = new_provider
+ metrics.set_meter_provider(new_provider) 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||
except Exception: | ||||||||||||||||||||||||||||||||||||||||||||||||
current_provider._genai_buckets_applied = True | ||||||||||||||||||||||||||||||||||||||||||||||||
except (ImportError, Exception): | ||||||||||||||||||||||||||||||||||||||||||||||||
pass |
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.
🛠️ Refactor suggestion
LLM operation duration view lacks explicit aggregation configuration.
Unlike other views, this method doesn't specify an
ExplicitBucketHistogramAggregation
, which means it will use the default histogram buckets. This inconsistency might lead to suboptimal bucket boundaries for duration metrics.Add explicit bucket configuration for consistency:
📝 Committable suggestion
🤖 Prompt for AI Agents