Standard Metrics - Outgoing Requests Per Second#729
Standard Metrics - Outgoing Requests Per Second#729reyang merged 96 commits intocensus-instrumentation:masterfrom
Conversation
|
This PR also includes the implementation of a thread context, named "is_exporter_thread", which is a flag set whenever the thread being run is one that has an exporter in it. This is to prevent spans and metrics being generated if the requests are being sent from an exporter. This includes ignoring span generation with applications configured to use opencensus-ext-requests and opencensus-ext-httplib, as well as metric collection from all opencensus exporters. |
| from opencensus.ext.azure.common.transport import TransportMixin | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
| if 'is_exporter_thread' not in RuntimeContext.snapshot().keys(): |
There was a problem hiding this comment.
I guess there is a better place for this instead of the log_exporter.
Going forward in OpenTelemetry we're trying to remove the requirement of having to register slots.
There was a problem hiding this comment.
Right. Instead of checking and changing the RuntimeContext for every exporter, we use a slot in the execution context now instead and simply import the module in each exporter.
| STANDARD_METRICS = [AvailableMemoryMetric, ProcessCPUMetric, | ||
| ProcessMemoryMetric, ProcessorTimeMetric] | ||
| STANDARD_METRICS = [AvailableMemoryMetric, DependencyRateMetric, | ||
| ProcessCPUMetric, ProcessMemoryMetric, |
There was a problem hiding this comment.
Personally I think it could be better to put each item on its own line, no strong opinion though.
There was a problem hiding this comment.
I like it better as well. Makes it less annoying to reformat and alphabetize.
| try: | ||
| # Check if request was sent from an exporter. If so, do not wrap. | ||
| disable_from_exporter = RuntimeContext.is_exporter_thread | ||
| if disable_from_exporter: |
There was a problem hiding this comment.
Minor, would it be better to put if RuntimeContext.is_exporter_thread: directly?
| _exporter_slot = RuntimeContext.register_slot('is_exporter_thread', False) | ||
| _tracer_slot = RuntimeContext.register_slot('tracer', noop_tracer.NoopTracer()) | ||
|
|
||
| def is_exporter_thread(): |
There was a problem hiding this comment.
Curious, why do we need this instead of using RuntimeContext.is_exporter_thread directly?
There was a problem hiding this comment.
-
There should only be a single way to read/modify the same piece of data. It would be quite awkward to read using
RuntimeContextand write usingexecution_context -
Consistency with the other
execution_contextslots.
There was a problem hiding this comment.
There should only be a single way to read/modify the same piece of data. It would be quite awkward to read using RuntimeContext and write using execution_context
This single way would be RuntimeContext.is_exporter_thread.
Consistency with the other execution_context slots.
We're in the direction to remove execution_context.
There was a problem hiding this comment.
It would be strange to have execution_context imported but not used.
There was a problem hiding this comment.
I see. Okay for this PR, we should clean this up once we say goodbye to execution_context.
There was a problem hiding this comment.
Please clean up
.
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
| # No retry policy, log output | ||
| logger.warning('Transient client side error %s.', ex) | ||
| return | ||
| finally: |
There was a problem hiding this comment.
It's great that we can say goodbye to this :)
4 participants
Part of [#695]. Standard metric for outgoing requests per second. Outgoing requests is under "dependencies".
Tracks requests made from application using the requests library. Other http libraries include urllib2 (python2 exclusive), urllib(python3 exclusive) and urllib3 (both) which might be introduced in the future.
Outgoing request rate is not part of Azure Monitor's standard metrics, so to see the data, view them under "customMetrics" in Kusto.