[go: up one dir, main page]

RFC: Workers: requests can have optional metadata

Pierrick Coudercrequested to merge
picdc@workers-with-metadata into master

What

This MR attach optional metadata when pushing requests to the worker. This is a generalization of 9f107d71 that only adds this (painfully) to the prevalidator requests.

Why

When profiling, especially to try to have consistent traces with Opentelemetry, it can be useful to propagate the scope of the current trace. Opentelemetry works with a "global" context that contains its current "scope": the current trace id, the last "span", etc. When pushing the request in a worker's queue, this request is handled concurrently in another context and the scope is lost. Such metadata would be used to encode the scope and push it alongside the request, so that the trace can remain consistent.

How

The trace is generic: it is encoded as a Jsonm value so that any profiling backend can use it to propagate its data. It could also serve other purposes. At the time of opening the MR, the change is as follows:

  • The worker internal messages contain their respective metadata
  • Metadatas are optional when pushing requests (to make the interface backward compatible)
  • Metadatas are handled by the Handler (as they are the one knowing what to do with them), so the interface is unfortunately not backward compatible anymore. Updating all the handlers in the repo is mechanical.

This is a generalization of a specific case for Opentelemetry. A halfway solution with 9f107d71 could be to simply make the worker use the Opentelemetry profiler (!16040 (merged)) directly and change metadata to Opentelemetry.Scope.t. As such:

  • Each message would contain its scope.
  • When pushing a request, take the scope and put it in the message.
  • When handling a new message, take its scope and update the "global context" of Opentelemetry.

It wouldn't require an API change of the worker, and the worker is responsible for maintaining the scope when the profiler is enabled.

Manually testing the MR

It's hard to test as is, but it is used in !16119 (merged). I'll write a test that shows metadata propagation once we are settled on a design.

Checklist

  • Document the interface of any function added or modified (see the coding guidelines)
  • Document any change to the user interface, including configuration parameters (see node configuration)
  • Provide automatic testing (see the testing guide).
  • For new features and bug fixes, add an item in the appropriate changelog (docs/protocols/alpha.rst for the protocol and the environment, CHANGES.rst at the root of the repository for everything else).
  • Select suitable reviewers using the Reviewers field below.
  • Select as Assignee the next person who should take action on that MR

Merge request reports