chore(tracing): deprecate ddtrace.trace.Pin #14361
Conversation
brettlangdon
requested review from
ZStriker19,
quinna-h,
emmettbutler and
mabdinur
|
|
brettlangdon
changed the title
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 265 ± 3 ms. The average import time from base is: 267 ± 2 ms. The import time difference between this PR and base is: -1.6 ± 0.1 ms. Import time breakdownThe following import paths have shrunk:
|
|
Great work, Brett! 👏 I have two questions about this PR. In the release notes it says:
IMHO, don’t you think this might be unclear to users? Shouldn’t we include an example in the Read the Docs?
Can we quantify the performance improvement? (not mandatory, only I'm curious) |
The only "Pin" examples I could find in the Read the Docs was for the contribution guidelines for a new integration. Otherwise none of our docs/integration docs state to use a Pin. Which is 🥳
Great question. I have a django load test app I have been using, a basic hello world + sqlite + in-memory cache, gunicorn with 4 workers, 50 concurrent users for 300k requests. Looking at my load test, django simple, and flask simple... we get this, "ish":
So it does vary widely, but being able to capture back up to 0.5 ms per request is pretty big, and that Pin overhead will increase for apps with more integrations, which make more db calls, etc. Ok... we probably won't be able to capture that whole 0.5-0.6 ms per-request since replacing Pin we might still have to do attribute look ups on config objects and etc, but reducing total function calls and object creation is definitely a solid/easy win (for something IMHO we don't need anymore) |
Performance SLOsCandidate: LANGPLAT-741/deprecate.pin (130f593) 🔵 No Baseline Data (24 suites)🔵 coreapiscenario - 12/12 (2 unstable)🔵 No baseline data available for this suite
|
There isn't much need for the Pin API anymore, we generally encourage environment variable or backend controlled configuration.
The big benefit of deprecating and removing this API is because of the performance impact it has on integrations, the operations to check for Pins is very slow, when generally it is expected people to use sane defaults or configure once via env variables.
Checklist
Reviewer Checklist