Conversation
Codeowners resolved as |
|
Performance SLOsComparing candidate dubloom/events-api (8f14d65) with baseline main (bae5178) 📈 Performance Regressions (2 suites)📈 iastaspectsospath - 24/24✅ ospathbasename_aspectTime: ✅ 498.636µs (SLO: <700.000µs 📉 -28.8%) vs baseline: 📈 +17.1% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.9% ✅ ospathbasename_noaspectTime: ✅ 431.047µs (SLO: <700.000µs 📉 -38.4%) vs baseline: -0.5% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +5.0% ✅ ospathjoin_aspectTime: ✅ 630.334µs (SLO: <700.000µs -10.0%) vs baseline: -0.3% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.9% ✅ ospathjoin_noaspectTime: ✅ 639.556µs (SLO: <700.000µs -8.6%) vs baseline: -0.8% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +5.0% ✅ ospathnormcase_aspectTime: ✅ 349.279µs (SLO: <700.000µs 📉 -50.1%) vs baseline: -0.2% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.8% ✅ ospathnormcase_noaspectTime: ✅ 357.991µs (SLO: <700.000µs 📉 -48.9%) vs baseline: ~same Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +5.0% ✅ ospathsplit_aspectTime: ✅ 492.841µs (SLO: <700.000µs 📉 -29.6%) vs baseline: -0.4% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.9% ✅ ospathsplit_noaspectTime: ✅ 500.184µs (SLO: <700.000µs 📉 -28.5%) vs baseline: -1.0% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.8% ✅ ospathsplitdrive_aspectTime: ✅ 378.470µs (SLO: <700.000µs 📉 -45.9%) vs baseline: +0.3% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.8% ✅ ospathsplitdrive_noaspectTime: ✅ 73.635µs (SLO: <700.000µs 📉 -89.5%) vs baseline: -0.2% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.8% ✅ ospathsplitext_aspectTime: ✅ 457.793µs (SLO: <700.000µs 📉 -34.6%) vs baseline: -0.8% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +5.0% ✅ ospathsplitext_noaspectTime: ✅ 464.517µs (SLO: <700.000µs 📉 -33.6%) vs baseline: ~same Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.9% 📈 telemetryaddmetric - 30/30✅ 1-count-metric-1-timesTime: ✅ 3.409µs (SLO: <20.000µs 📉 -83.0%) vs baseline: 📈 +16.5% Memory: ✅ 35.527MB (SLO: <38.000MB -6.5%) vs baseline: +4.9% ✅ 1-count-metrics-100-timesTime: ✅ 204.709µs (SLO: <220.000µs -7.0%) vs baseline: -0.2% Memory: ✅ 35.586MB (SLO: <38.000MB -6.4%) vs baseline: +5.0% ✅ 1-distribution-metric-1-timesTime: ✅ 3.274µs (SLO: <20.000µs 📉 -83.6%) vs baseline: +0.1% Memory: ✅ 35.586MB (SLO: <38.000MB -6.4%) vs baseline: +5.0% ✅ 1-distribution-metrics-100-timesTime: ✅ 213.497µs (SLO: <230.000µs -7.2%) vs baseline: -0.7% Memory: ✅ 35.527MB (SLO: <38.000MB -6.5%) vs baseline: +4.9% ✅ 1-gauge-metric-1-timesTime: ✅ 2.132µs (SLO: <20.000µs 📉 -89.3%) vs baseline: ~same Memory: ✅ 35.586MB (SLO: <38.000MB -6.4%) vs baseline: +5.3% ✅ 1-gauge-metrics-100-timesTime: ✅ 136.181µs (SLO: <150.000µs -9.2%) vs baseline: +0.1% Memory: ✅ 35.488MB (SLO: <38.000MB -6.6%) vs baseline: +5.0% ✅ 1-rate-metric-1-timesTime: ✅ 3.024µs (SLO: <20.000µs 📉 -84.9%) vs baseline: -0.5% Memory: ✅ 35.448MB (SLO: <38.000MB -6.7%) vs baseline: +4.5% ✅ 1-rate-metrics-100-timesTime: ✅ 218.344µs (SLO: <250.000µs 📉 -12.7%) vs baseline: -0.6% Memory: ✅ 35.488MB (SLO: <38.000MB -6.6%) vs baseline: +4.7% ✅ 100-count-metrics-100-timesTime: ✅ 20.470ms (SLO: <22.000ms -7.0%) vs baseline: -0.5% Memory: ✅ 35.606MB (SLO: <38.000MB -6.3%) vs baseline: +5.0% ✅ 100-distribution-metrics-100-timesTime: ✅ 2.229ms (SLO: <2.550ms 📉 -12.6%) vs baseline: +0.3% Memory: ✅ 35.645MB (SLO: <38.000MB -6.2%) vs baseline: +5.3% ✅ 100-gauge-metrics-100-timesTime: ✅ 1.390ms (SLO: <1.550ms 📉 -10.3%) vs baseline: -0.2% Memory: ✅ 35.566MB (SLO: <38.000MB -6.4%) vs baseline: +5.0% ✅ 100-rate-metrics-100-timesTime: ✅ 2.219ms (SLO: <2.550ms 📉 -13.0%) vs baseline: -0.8% Memory: ✅ 35.488MB (SLO: <38.000MB -6.6%) vs baseline: +4.7% ✅ flush-1-metricTime: ✅ 4.587µs (SLO: <20.000µs 📉 -77.1%) vs baseline: +0.3% Memory: ✅ 35.665MB (SLO: <38.000MB -6.1%) vs baseline: +5.0% ✅ flush-100-metricsTime: ✅ 175.647µs (SLO: <250.000µs 📉 -29.7%) vs baseline: +0.1% Memory: ✅ 35.507MB (SLO: <38.000MB -6.6%) vs baseline: +4.6% ✅ flush-1000-metricsTime: ✅ 2.181ms (SLO: <2.500ms 📉 -12.8%) vs baseline: -0.1% Memory: ✅ 36.353MB (SLO: <38.750MB -6.2%) vs baseline: +4.9% 🟡 Near SLO Breach (1 suite)🟡 tracer - 6/6✅ largeTime: ✅ 31.647ms (SLO: <32.950ms -4.0%) vs baseline: -0.2% Memory: ✅ 36.805MB (SLO: <39.250MB -6.2%) vs baseline: +5.1% ✅ mediumTime: ✅ 3.123ms (SLO: <3.200ms -2.4%) vs baseline: -0.3% Memory: ✅ 35.291MB (SLO: <38.750MB -8.9%) vs baseline: +5.2% ✅ smallTime: ✅ 364.712µs (SLO: <370.000µs 🟡 -1.4%) vs baseline: +3.7% Memory: ✅ 35.193MB (SLO: <38.750MB -9.2%) vs baseline: +4.7%
|
947a07f to
8d7776e
Compare
wconti27
left a comment
There was a problem hiding this comment.
Have some open questions for you.
| def test_basic_context_event(test_spans): | ||
| """Test that a basic ContextEvent creates a span with the correct name""" | ||
|
|
||
| class TestContextEvent(ContextEvent): | ||
| event_name = "test.event" | ||
| span_kind = "kind" | ||
| component = "component" | ||
| span_type = "type" | ||
|
|
||
| def __new__(cls): | ||
| return cls.create(span_name="test") | ||
|
|
||
| with core.context_with_event(TestContextEvent()): | ||
| pass | ||
|
|
||
| test_spans.assert_span_count(1) | ||
| span = test_spans.spans[0] | ||
| assert span.name == "test" |
There was a problem hiding this comment.
We should always set span kind and component and not rely on child classes setting those tags.
There was a problem hiding this comment.
span_kind and component depends on the Integration categories so it has to be a child class that sets those tags
mabdinur
left a comment
There was a problem hiding this comment.
Overall, I like the approach. We will need to migrate a few integrations before we confident this model fits in our use case. Can we open up a PR or two against this branch?
ddtrace/internal/core/events.py
Outdated
| span_kind: Optional[str] = None | ||
| component: Optional[str] = None | ||
| span_type: Optional[str] = None | ||
| call_trace: bool = False |
There was a problem hiding this comment.
nit: Instead of using call_trace we can always just use tracer._start_span. By default child_of would be set to the current active and activate would be set to True.
Essentially we can replace call_trace (which is a bit confusing) with two arguments: activate and child_of. But we can save this refactor for a future PR
There was a problem hiding this comment.
As we are using _start_span, we need to use call_trace and distributed_ctx instead. We could change that by changing _start_span but I don't think this is a high priority
1aa7a5f to
2f1555c
Compare
79b7139 to
40204f7
Compare
|
Do we/will we have a performance analysis for the impact of this abstraction? |
8f14d65 to
a9c824e
Compare
Code done, still have to put a bit of documentation here and there