-
Notifications
You must be signed in to change notification settings - Fork 467
chore(telemetry): remove skipped and remove some logs from telemetry #15192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 213 ± 3 ms. The average import time from base is: 217 ± 6 ms. The import time difference between this PR and base is: -4.1 ± 0.2 ms. Import time breakdownThe following import paths have shrunk:
|
Performance SLOsComparing candidate dubloom/fix-error-logs-remediation (a97d6ac) with baseline main (83a032e) 🟡 Near SLO Breach (6 suites)🟡 djangosimple - 30/30✅ appsecTime: ✅ 20.489ms (SLO: <22.300ms -8.1%) vs baseline: +0.1% Memory: ✅ 66.172MB (SLO: <67.000MB 🟡 -1.2%) vs baseline: +5.0% ✅ exception-replay-enabledTime: ✅ 1.337ms (SLO: <1.450ms -7.8%) vs baseline: -0.3% Memory: ✅ 64.242MB (SLO: <67.000MB -4.1%) vs baseline: +4.9% ✅ iastTime: ✅ 20.502ms (SLO: <22.250ms -7.9%) vs baseline: +0.2% Memory: ✅ 66.336MB (SLO: <67.000MB 🟡 -1.0%) vs baseline: +5.1% ✅ profilerTime: ✅ 15.535ms (SLO: <16.550ms -6.1%) vs baseline: ~same Memory: ✅ 54.015MB (SLO: <54.500MB 🟡 -0.9%) vs baseline: +4.9% ✅ resource-renamingTime: ✅ 20.564ms (SLO: <21.750ms -5.5%) vs baseline: +0.4% Memory: ✅ 66.109MB (SLO: <67.000MB 🟡 -1.3%) vs baseline: +4.7% ✅ span-code-originTime: ✅ 25.445ms (SLO: <28.200ms -9.8%) vs baseline: ~same Memory: ✅ 67.272MB (SLO: <69.500MB -3.2%) vs baseline: +3.0% ✅ tracerTime: ✅ 20.524ms (SLO: <21.750ms -5.6%) vs baseline: +0.3% Memory: ✅ 66.174MB (SLO: <67.000MB 🟡 -1.2%) vs baseline: +5.0% ✅ tracer-and-profilerTime: ✅ 22.713ms (SLO: <23.500ms -3.3%) vs baseline: -0.1% Memory: ✅ 67.719MB (SLO: <68.000MB 🟡 -0.4%) vs baseline: +4.7% ✅ tracer-dont-create-db-spansTime: ✅ 19.311ms (SLO: <21.500ms 📉 -10.2%) vs baseline: +0.2% Memory: ✅ 66.159MB (SLO: <67.000MB 🟡 -1.3%) vs baseline: +5.0% ✅ tracer-minimalTime: ✅ 16.618ms (SLO: <17.500ms -5.0%) vs baseline: ~same Memory: ✅ 65.992MB (SLO: <67.000MB 🟡 -1.5%) vs baseline: +4.6% ✅ tracer-nativeTime: ✅ 20.398ms (SLO: <21.750ms -6.2%) vs baseline: ~same Memory: ✅ 67.849MB (SLO: <72.500MB -6.4%) vs baseline: +4.9% ✅ tracer-no-cachesTime: ✅ 18.458ms (SLO: <19.650ms -6.1%) vs baseline: -0.2% Memory: ✅ 66.208MB (SLO: <67.000MB 🟡 -1.2%) vs baseline: +5.0% ✅ tracer-no-databasesTime: ✅ 18.812ms (SLO: <20.100ms -6.4%) vs baseline: +0.4% Memory: ✅ 65.815MB (SLO: <67.000MB 🟡 -1.8%) vs baseline: +4.5% ✅ tracer-no-middlewareTime: ✅ 20.190ms (SLO: <21.500ms -6.1%) vs baseline: -0.1% Memory: ✅ 66.227MB (SLO: <67.000MB 🟡 -1.2%) vs baseline: +5.1% ✅ tracer-no-templatesTime: ✅ 20.295ms (SLO: <22.000ms -7.8%) vs baseline: ~same Memory: ✅ 66.129MB (SLO: <67.000MB 🟡 -1.3%) vs baseline: +5.1% 🟡 errortrackingdjangosimple - 6/6✅ errortracking-enabled-allTime: ✅ 18.085ms (SLO: <19.850ms -8.9%) vs baseline: +0.2% Memory: ✅ 66.089MB (SLO: <66.500MB 🟡 -0.6%) vs baseline: +4.9% ✅ errortracking-enabled-userTime: ✅ 18.134ms (SLO: <19.400ms -6.5%) vs baseline: +0.5% Memory: ✅ 66.129MB (SLO: <66.500MB 🟡 -0.6%) vs baseline: +4.8% ✅ tracer-enabledTime: ✅ 18.068ms (SLO: <19.450ms -7.1%) vs baseline: -0.1% Memory: ✅ 66.027MB (SLO: <66.500MB 🟡 -0.7%) vs baseline: +5.1% 🟡 errortrackingflasksqli - 6/6✅ errortracking-enabled-allTime: ✅ 2.076ms (SLO: <2.300ms -9.7%) vs baseline: +0.4% Memory: ✅ 52.632MB (SLO: <53.500MB 🟡 -1.6%) vs baseline: +4.9% ✅ errortracking-enabled-userTime: ✅ 2.066ms (SLO: <2.250ms -8.2%) vs baseline: +0.2% Memory: ✅ 52.632MB (SLO: <53.500MB 🟡 -1.6%) vs baseline: +4.8% ✅ tracer-enabledTime: ✅ 2.065ms (SLO: <2.300ms 📉 -10.2%) vs baseline: +0.3% Memory: ✅ 52.612MB (SLO: <53.500MB 🟡 -1.7%) vs baseline: +4.8% 🟡 flasksimple - 18/18✅ appsec-getTime: ✅ 4.601ms (SLO: <4.750ms -3.1%) vs baseline: +0.4% Memory: ✅ 62.401MB (SLO: <65.000MB -4.0%) vs baseline: +5.0% ✅ appsec-postTime: ✅ 6.633ms (SLO: <6.750ms 🟡 -1.7%) vs baseline: -0.3% Memory: ✅ 62.324MB (SLO: <65.000MB -4.1%) vs baseline: +5.0% ✅ appsec-telemetryTime: ✅ 4.589ms (SLO: <4.750ms -3.4%) vs baseline: ~same Memory: ✅ 62.342MB (SLO: <65.000MB -4.1%) vs baseline: +4.8% ✅ debuggerTime: ✅ 1.855ms (SLO: <2.000ms -7.2%) vs baseline: -0.3% Memory: ✅ 45.375MB (SLO: <47.000MB -3.5%) vs baseline: +5.4% ✅ iast-getTime: ✅ 1.854ms (SLO: <2.000ms -7.3%) vs baseline: -0.6% Memory: ✅ 42.119MB (SLO: <49.000MB 📉 -14.0%) vs baseline: +4.8% ✅ profilerTime: ✅ 1.921ms (SLO: <2.100ms -8.5%) vs baseline: +0.6% Memory: ✅ 46.749MB (SLO: <47.000MB 🟡 -0.5%) vs baseline: +4.9% ✅ resource-renamingTime: ✅ 3.372ms (SLO: <3.650ms -7.6%) vs baseline: +0.2% Memory: ✅ 52.658MB (SLO: <53.500MB 🟡 -1.6%) vs baseline: +5.0% ✅ tracerTime: ✅ 3.353ms (SLO: <3.650ms -8.1%) vs baseline: -0.3% Memory: ✅ 52.467MB (SLO: <53.500MB 🟡 -1.9%) vs baseline: +4.4% ✅ tracer-nativeTime: ✅ 3.353ms (SLO: <3.650ms -8.1%) vs baseline: ~same Memory: ✅ 54.055MB (SLO: <60.000MB -9.9%) vs baseline: +4.7% 🟡 flasksqli - 6/6✅ appsec-enabledTime: ✅ 3.975ms (SLO: <4.200ms -5.4%) vs baseline: ~same Memory: ✅ 62.423MB (SLO: <66.000MB -5.4%) vs baseline: +4.9% ✅ iast-enabledTime: ✅ 2.450ms (SLO: <2.800ms 📉 -12.5%) vs baseline: +0.5% Memory: ✅ 58.845MB (SLO: <60.000MB 🟡 -1.9%) vs baseline: +4.8% ✅ tracer-enabledTime: ✅ 2.062ms (SLO: <2.250ms -8.4%) vs baseline: +0.3% Memory: ✅ 52.612MB (SLO: <54.500MB -3.5%) vs baseline: +4.7% 🟡 recursivecomputation - 8/8✅ deepTime: ✅ 309.218ms (SLO: <320.950ms -3.7%) vs baseline: ~same Memory: ✅ 32.657MB (SLO: <34.500MB -5.3%) vs baseline: +4.4% ✅ deep-profiledTime: ✅ 327.247ms (SLO: <359.150ms -8.9%) vs baseline: -0.4% Memory: ✅ 38.597MB (SLO: <39.000MB 🟡 -1.0%) vs baseline: +4.7% ✅ mediumTime: ✅ 7.001ms (SLO: <7.400ms -5.4%) vs baseline: -0.2% Memory: ✅ 31.968MB (SLO: <34.000MB -6.0%) vs baseline: +4.9% ✅ shallowTime: ✅ 0.946ms (SLO: <1.050ms -9.9%) vs baseline: +0.6% Memory: ✅ 32.047MB (SLO: <34.000MB -5.7%) vs baseline: +5.1%
|
| trace_id, span_id, trace_flag = _TraceContext._get_traceparent_values(tp) | ||
| except (ValueError, AssertionError): | ||
| log.exception("received invalid w3c traceparent: %s ", tp) | ||
| log.exception("received invalid w3c traceparent: %s ", tp, extra={"send_to_telemetry": False}) |
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.
should this be a log.exception? maybe warning at best?
probably depends on whether it is an internal app talking to another internal app, or something exposed to the public internet.
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.
To be honest, I have no opinion. If you think this would be better as a log.warning, I can make that change
| if getattr(record, "send_to_telemetry", None) in (None, True): | ||
| self.telemetry_writer.add_error_log(record.msg, record.exc_info) | ||
| # we do not want to send the [x skipped] part to telemetry | ||
| msg = re.sub(r"\s*\[\d+ skipped\]$", "", record.msg) |
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.
can we handle this on intake?
this means we'll only get this deduping fix once all clients have upgraded to this version of the tracer or newer.
also, what if we change the log message format, then we are stuck doing this again.
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.
I agree that this is more robust, do you know who I should talk to in order to fix that ?
Now we send error logs to telemetry, we've seen two unexpected behavior: