-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Remove "a/" from tracing keys in special-key-space #4150
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
Conversation
|
LGTM. |
I don't think there will be more keys added to the top level |
yeah, adding |
|
I'm speculating about possible future features -- however, in general I think most span state should be internal and the transaction shouldn't need to access it. In that case, the above example wouldn't need to be added. @sfc-gh-mpilman do you have thoughts on this? |
|
Sorry for misleading, I just confirm the contract with Andrew for what is a behavior change in special keys. |
Changes in this PR:
a/fromtracingnamespace in special-key-space. For example,\xff\xff/tracing/a/transaction_idbecomes\xff\xff/tracing/transaction_id.Style
git clang-format).Performance
All CPU-hot paths are well optimized.The proper containers are used (for examplestd::vectorvsVectorRef).There are no new knownSlowTasktraces.Testing
If there are new parameters or knobs, different values are tested in simulation.ASSERT,ASSERT_WE_THINK, andTESTmacros are added in appropriate places.Unit tests were added for new algorithms and data structure that make sense to unit-test