NH-60984 Add Otel Python SDK-based histogram for response_time #201
Conversation
tammy-baylis-swi
changed the title
tammy-baylis-swi
force-pushed
the
NH-60984-add-otlp-meters
branch
from
81165f4 to
dbb6bbf
Compare
This reverts commit fa97c56.
tammy-baylis-swi
changed the title
tammy-baylis-swi
force-pushed
the
NH-60984-add-otlp-meters
branch
from
4a39ae2 to
491cb99
Compare
tammy-baylis-swi
changed the title
| @@ -5,4 +5,4 @@ | |||
| # Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. | |||
|
|
|||
| """Version of SolarWinds Custom-Distro for OpenTelemetry agents""" | |||
| __version__ = "0.18.0.1" | |||
| __version__ = "0.18.0.6" | |||
There was a problem hiding this comment.
I did a few testreleases to create APM Python lambda layer from testpypi and do some testing there.
There was a problem hiding this comment.
Thank you for the write up and digestable PR as always @tammy-baylis-swi! I focused on the areas not marked by TODO and left a few thoughts.
solarwinds_apm/apm_meter_manager.py
Outdated
| def __init__(self, **kwargs: int) -> None: | ||
| # Returns a named `Meter` to handle instrument creation. | ||
| # A convenience wrapper for MeterProvider.get_meter | ||
| self.meter = metrics.get_meter("sw.apm.sampling.metrics") |
There was a problem hiding this comment.
Ah, the sw.apm.sampling.metrics instrumentation scope name should only be given to our "request counter" metrics, it tells the collector to send them to sample rate hopper. For the "response time" metrics we haven't talked about what to set, but I see @cleverchuk currently uses sw.apm.request.metrics which makes sense, I'll start a discussion and get this in our spec.
solarwinds_apm/apm_noop.py
Outdated
| class SolarWindsMeterManager: | ||
| def __init__(self, *args, **kwargs): | ||
| self.meter = None | ||
| self.response_time = OtelHistogram() |
There was a problem hiding this comment.
Minor curiosity, whether the OTel NoOpHistogram can be used instead.
|
|
||
| # support ssa and conform to Otel proto common_pb2 | ||
| meter_attrs = { | ||
| "sw.service_name": self.service_name, |
There was a problem hiding this comment.
this is to conform to the SWO response time metric tagging, right? If so, sw.service_name isn't needed because Python doesn't support service name override.
There was a problem hiding this comment.
Ahhhh. Removed in f432626. Past Tammy also might have added it when trying to correlate metrics with service, before you found that it was that resource attribute needed instead!
| in nanoseconds (ns). OTel span start/end_time are optional.""" | ||
| if not start_time or not end_time: | ||
| return 0 | ||
| return int((end_time - start_time) // 1e6) |
There was a problem hiding this comment.
For the apm-proto metrics we definitely needed to convert to microseconds, but OTel metrics model seems to support nanoseconds so how about let's not do any conversion and pass in just the start/end delta as-is and see what we get :)
There was a problem hiding this comment.
|
Thank you for first pass @cheempz ! I've addressed them and posted a question about the start/end delta conversion if you could please take another look. |
There was a problem hiding this comment.
LGTM, thank you for the revisit and experiments @tammy-baylis-swi! I'll follow up with other teams on the unconverted nanosecond presentation in SWO :)
This adds a new SolarWinds
MeterManageras a place to store any Otel Python meters initialized at instrumented service startup. In this iteration we add one Otel Python SDK Histogram to represent thetrace.service.response_timederived/"inbound" metric in SWO.Example graphs of how this looks are screenshot in this doc: https://swicloud.atlassian.net/wiki/spaces/NIT/pages/3792503086/PoC+Otel+Metrics+to+recreate+Inbound+Metrics#trace.service.response_time
Also adds a new SolarWinds
OTLPMetricsSpanProcessortorecordto the histogram at the end of every service entry span. Right now this processor duplicates some of what theInboundMetricsSpanProcessordoes to set the metrics attributesw.transaction.Configurator is updated to initialized all of the above at instrumented service startup. Span processors are registered in order of add_span_processor calls so
InboundMetricsSpanProcessoris first andOTLPMetricsSpanProcessoris second when on_end is called.User needs to set
SW_APM_EXPERIMENTAL_OTEL_COLLECTORas per #200 for the histogram to be created and recorded to. If not set, metrics instruments are not created nor is the span processor for otlp metrics recording published.Next steps: