Implement attach rate statsbeat metrics #1053
Conversation
lzchen
requested review from
a team,
aabmass,
hectorhdzg and
songy23
as code owners
contrib/opencensus-ext-azure/opencensus/ext/azure/metrics_exporter/__init__.py
Show resolved
Hide resolved
| @@ -1,132 +0,0 @@ | |||
| # Copyright 2019, OpenCensus Authors | |||
There was a problem hiding this comment.
Why removing heartbeat?, I believe is still relevant and is not being replaced by Statsbeat
There was a problem hiding this comment.
Heartbeat was never fully implemented/committed for OpenCensus Python. There are no current plans to implement them either.
contrib/opencensus-ext-azure/opencensus/ext/azure/log_exporter/__init__.py
Outdated
Show resolved
Hide resolved
| if self._is_stats: | ||
| envelope.name = "Statsbeat" | ||
| else: | ||
| envelope.name = "Microsoft.ApplicationInsights.Metric" |
There was a problem hiding this comment.
You can save payload by sending "Metric" directly for regular custom metrics.
There was a problem hiding this comment.
Possibly. The benefit of saving the payload doesn't really outweigh the need to change unrelated code.
contrib/opencensus-ext-azure/opencensus/ext/azure/metrics_exporter/__init__.py
Show resolved
Hide resolved
| from opencensus.ext.azure.metrics_exporter import MetricsExporter | ||
| from opencensus.ext.azure.metrics_exporter.heartbeat_metrics.heartbeat import ( | ||
| HeartbeatMetric, | ||
| from opencensus.ext.azure.metrics_exporter.statsbeat_metrics.statsbeat import ( |
There was a problem hiding this comment.
Statsbeat is not a replacement of heartbeat. Might want to keep Hearbeat metrics. they're very diff.
There was a problem hiding this comment.
i realized statsbeat_metrics.statsbeat is not a package name.. rather the class name.
| properties.append(LabelKey("runtimeVersion", 'Python version')) | ||
| properties.append(LabelKey("os", 'os of application being instrumented')) | ||
| properties.append(LabelKey("language", 'Python')) | ||
| properties.append(LabelKey("version", 'version of exporter package')) |
There was a problem hiding this comment.
version is for python sdk's verison.. it's not clear to me if they're the same thing?
There was a problem hiding this comment.
Yes. We are using sdkVersion to represent this. For OpenCensus Python, that would look like pyX:ocY:extZ, X being the python version, Y being the OpenCensus version and Z being the exporter version.
...ib/opencensus-ext-azure/opencensus/ext/azure/metrics_exporter/statsbeat_metrics/statsbeat.py
Outdated
Show resolved
Hide resolved
...ib/opencensus-ext-azure/opencensus/ext/azure/metrics_exporter/statsbeat_metrics/statsbeat.py
Outdated
Show resolved
Hide resolved
| return properties | ||
|
|
||
|
|
||
| class _StatsbeatMetrics: |
There was a problem hiding this comment.
so that it's easier to control enablement/disablement.
There was a problem hiding this comment.
If enablement/disablement is done through environment variables, separate classes will be unnecessary. Will probably revisit once we fully spec out enable/disable.
| properties.append(LabelValue("python")) # language | ||
| # version | ||
| properties.append( | ||
| LabelValue(azure_monitor_context['ai.internal.sdkVersion'])) |
There was a problem hiding this comment.
should customMetrics handle this by default?
There was a problem hiding this comment.
@heyams I'm not sure I understand. Handle what by default? customMetrics is an Azure concept. LabelValue is an OpenCensus metrics concept.
There was a problem hiding this comment.
your telemetry client (not sure how you call it) should handle that for all telemetries to Breeze. this is required for all telemetries sent to breeze. I'm saying you should not do it specifically for statsbeat.
There was a problem hiding this comment.
Yes the this is already being handled here which populates the tags dict in breeze. The logic here in specific for stats because we also want version to be populated as a custom_dimension correct?
|
curious to know how you handle batching in python.. statsbeat's endpoint is diff from the customer's ikey endpoint.. all statsbeat will be batched together and send to its own endpoint... using customer's endpoint to send statsbeat will result failure. |
|
Another suggestion: please make statsbeat connection string configurable so that your testing data will go to your testing statsbeat endpoint. and you can control interval for faster testing. |
Added an environment variable to configure this. |
...ib/opencensus-ext-azure/opencensus/ext/azure/metrics_exporter/statsbeat_metrics/statsbeat.py
Show resolved
Hide resolved
...rib/opencensus-ext-azure/opencensus/ext/azure/metrics_exporter/statsbeat_metrics/__init__.py
Show resolved
Hide resolved
...rib/opencensus-ext-azure/opencensus/ext/azure/metrics_exporter/statsbeat_metrics/__init__.py
Outdated
Show resolved
Hide resolved
@heyams |
3 participants
Implement statsbeat feature, only supporting attach rate metrics.
NOTE:
Heartbeatmetrics was partially implemented previously for attach rate metrics, but now is encapsulated into stats, hence the rename.TODO:
Network metrics
Feature metrics
Secure with AAD
@heyams
@hectorhdzg