Introduce Gitlab::Instrumentation::Storage
What does this MR do and why?
See #409951 (closed), supports !118614 (closed).
Introduces Gitlab::Instrumentation::Storage
as a light-weight abstraction over SafeRequestStore
, used purely when storing data for purposes of instrumentation (see also InstrumentationHelper
). It uses a subset of the SafeRequestStore
interface, but stores all values written to a dedicated bucket.
This allows us to easily throw out instrumentation data when instrumenting more than one unit of work. This is necessary, for example, when dealing with websocket pushes instead of HTTP requests, where we want to instrument not just one thing (the request) but N things (upstream transmissions.)
Not a user facing change.
Immediate benefits we get from this:
- An intention revealing interface for writing and reading instrumentation data. While this still uses
RequestStore
underneath, it will be easier to swap this out later. - Segregation of instrumentation data from other data accumulated in
RequestStore
. - Better discovery of test issues where stubbing methods on
RequestStore
for instrumentation purposes had unintentional side-effects on any other code reading and writing toRequestStore
.
NOTE:
- Due to scope creep, I decided not to use sub-buckets within
instrumentation
as described in the issue. The safest and smallest iteration was to simply use a single parent bucket, which is also enough to solve the immediate problem of segregating instrumentation from other request data. - It is understood that this is still not a great abstraction; the idea of "request based storage" starts to fall apart in the context of websockets. Ideally, we would have truly segregated concepts of instrumentation storage and per-request storage, but that is a much larger change, and I see this MR as moving as one step in the right direction. I filed gitlab-org/ruby/gems/labkit-ruby#38 to track that work.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
How to set up and validate locally
Nothing should change observably. These data are collected and logged to e.g. development_json
. Presence of these data plus green tests should be good enough to see whether this works correctly.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #409951 (closed)