This repository was archived by the owner on Nov 16, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 16
Implement standard metrics for http dependency telemetry #125
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
# Copyright (c) Microsoft Corporation. All rights reserved. | ||
# Licensed under the MIT License. | ||
# pylint: disable=import-error | ||
# pylint: disable=no-member | ||
# pylint: disable=no-name-in-module | ||
import time | ||
|
||
import requests | ||
from opentelemetry import metrics | ||
from opentelemetry.instrumentation.requests import RequestsInstrumentor | ||
from opentelemetry.sdk.metrics import MeterProvider | ||
|
||
from azure_monitor import AzureMonitorMetricsExporter | ||
|
||
# Use the default sdk implementation | ||
metrics.set_meter_provider(MeterProvider(stateful=False)) | ||
|
||
# Track telemetry from the requests library | ||
RequestsInstrumentor().instrument() | ||
meter = RequestsInstrumentor().meter | ||
exporter = AzureMonitorMetricsExporter( | ||
connection_string="InstrumentationKey=<INSTRUMENTATION KEY HERE>" | ||
) | ||
# Export standard metrics from requests library to Azure Monitor | ||
metrics.get_meter_provider().start_pipeline(meter, exporter, 5) | ||
|
||
for x in range(10): | ||
for y in range(10): | ||
requests.get("http://example.com") | ||
time.sleep(2) | ||
time.sleep(5) | ||
|
||
input("Press any key to exit...") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,10 @@ class AzureMonitorMetricsExporter(BaseExporter, MetricsExporter): | |
options: :doc:`export.options` to allow configuration for the exporter | ||
""" | ||
|
||
def __init__(self, **options): | ||
super().__init__(**options) | ||
self.add_telemetry_processor(standard_metrics_processor) | ||
|
||
def export( | ||
self, metric_records: Sequence[MetricRecord] | ||
) -> MetricsExportResult: | ||
|
@@ -73,13 +77,19 @@ def _metric_to_envelope( | |
) | ||
envelope.name = "Microsoft.ApplicationInsights.Metric" | ||
value = 0 | ||
_min = None | ||
_max = None | ||
count = None | ||
metric = metric_record.instrument | ||
if isinstance(metric, ValueObserver): | ||
# mmscl | ||
value = metric_record.aggregator.checkpoint.last | ||
elif isinstance(metric, ValueRecorder): | ||
# mmsc | ||
value = metric_record.aggregator.checkpoint.count | ||
value = metric_record.aggregator.checkpoint.sum | ||
_min = metric_record.aggregator.checkpoint.min | ||
_max = metric_record.aggregator.checkpoint.max | ||
count = metric_record.aggregator.checkpoint.count | ||
else: | ||
# sum or lv | ||
value = metric_record.aggregator.checkpoint | ||
|
@@ -90,6 +100,9 @@ def _metric_to_envelope( | |
ns=metric.description, | ||
name=metric.name, | ||
value=value, | ||
min=_min, | ||
max=_max, | ||
count=count, | ||
kind=protocol.DataPointType.MEASUREMENT.value, | ||
) | ||
|
||
|
@@ -99,3 +112,43 @@ def _metric_to_envelope( | |
data = protocol.MetricData(metrics=[data_point], properties=properties) | ||
envelope.data = protocol.Data(base_data=data, base_type="MetricData") | ||
return envelope | ||
|
||
|
||
def standard_metrics_processor(envelope): | ||
data = envelope.data.base_data | ||
if data.metrics: | ||
properties = {} | ||
point = data.metrics[0] | ||
if point.name == "http.client.duration": | ||
point.name = "Dependency duration" | ||
point.kind = protocol.DataPointType.AGGREGATION.value | ||
properties["_MS.MetricId"] = "dependencies/duration" | ||
properties["_MS.IsAutocollected"] = "True" | ||
properties["cloud/roleInstance"] = utils.azure_monitor_context.get( | ||
"ai.cloud.roleInstance" | ||
) | ||
properties["cloud/roleName"] = utils.azure_monitor_context.get( | ||
"ai.cloud.role" | ||
) | ||
properties["Dependency.Success"] = "False" | ||
if data.properties.get("http.status_code"): | ||
try: | ||
code = int(data.properties.get("http.status_code")) | ||
if 200 <= code < 400: | ||
properties["Dependency.Success"] = "True" | ||
except ValueError: | ||
pass | ||
# TODO: Check other properties if url doesn't exist | ||
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. What kind of other properties? 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. |
||
properties["dependency/target"] = data.properties.get("http.url") | ||
properties["Dependency.Type"] = "HTTP" | ||
properties["dependency/resultCode"] = data.properties.get( | ||
"http.status_code" | ||
) | ||
# Won't need this once Azure Monitor supports histograms | ||
# We can't actually get the individual buckets because the bucket | ||
# collection must happen on the SDK side | ||
properties["dependency/performanceBucket"] = "" | ||
# TODO: OT does not have this in semantic conventions for trace | ||
properties["operation/synthetic"] = "" | ||
# TODO: Add other std. metrics as implemented | ||
data.properties = properties |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Looks like we are getting more AI properties like this one and others, it would be good to have them somewhere as constants to avoid bugs regarding typos
Uh oh!
There was an error while loading. Please reload this page.
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.
Moving AI properties to a constants file I feel implies that they are a "global" accessible variable. These properties are only relevant to standard metric implementation and so should exist only in that scope (within the metrics file). Perhaps it will be more suitable at the top of the module as constants, but until there are other implementations in other files related to standard metrics, or multiple uses for the constants, I don't see this as much of an improvement.
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 is not blocking comment, is up to you to handle this, having the constants in the top of the file let you quickly realize we are using these and in case something change is easier to update