workflow-step-api-plugin icon indicating copy to clipboard operation
workflow-step-api-plugin copied to clipboard

Add `SynchronousNonBlockingStepExecution.ExecutorServiceAugmentor` extension point

Open jgreffe opened this issue 5 months ago • 11 comments

Allow the executorService to be augmented. This avoids having to use reflection by dependent plugins.

See https://github.com/jenkinsci/opentelemetry-plugin/pull/1139 for the consumer.

Updated with comment from @dwnusbaum

opentelemetry wants to make it possible for other plugins to augment the traces produced for Jenkins builds. See jenkinsci/opentelemetry-plugin#896 and jenkinsci/opentelemetry-plugin#909 for more context. See jenkinsci/junit-sql-storage-plugin#413 for an example of a plugin using the OTel context to emit custom spans as part of the junit step.

In order for this to work, plugins need access to an OTel Context in their build steps and/or related code so that they can produce a Span with the correct parent. By default, OTel contexts are stored as a ThreadLocal variable. opentelemetry's synchronous GraphListener implementation sets up this context on the CPS VM thread when a new step starts. For Pipeline steps with fully synchronous StepExecutions, that is enough for context propagation to work correctly within the extent of StepExecution.start. However, for steps whose execution runs on another thread, even if they complete synchronously from the perspective of the Pipeline (e.g. SynchronousNonBlockingStepExecution, GeneralNonBlockingStepExecution), the context will be unavailable, and so any created OTel span will not have the correct parent.

opentelemetry plugin currently handles this case by reflectively modifying SynchronousNonBlockingStepExecution.executorService to wrap it with io.opentelemetry.context.Context.taskWrapping, which automatically propagates the thread local OTel context to tasks run on the executor service. This works, but this kind of reflection is brittle, and we would like to support this use case in a more maintainable way. This PR currently just exposes a @Restricted(Beta.class) API to allow opentelemetry to augment SynchronousNonBlockingStepExecution.executorService with Context.taskWrapping without using reflection.

There may be other ways to accomplish the same use case, but we have not explored them at this time: For example:

  • Perhaps we should instead encourage plugins with build steps that want to support tracing to always look up their parent span using these methods and then set up the context manually as needed? For junit-sql-storage this would mean that the junit step would need to be updated to look up the span and then set up the thread local context for methods in the custom storage provider.
  • Similarly, could we somehow inject the OTel Context into the StepContext for each step automatically, and then have some kind of method like OtelPlugin.useContext(StepContext) that sets up the thread local for plugins that need it? This would only work for Pipeline Steps, but maybe that's ok?
  • There is a ContextStorageProvider API. Perhaps we could implement this in a way that works better for our use case? EDIT: Or perhaps we could invert the dependency as described in: https://github.com/jenkinsci/workflow-step-api-plugin/pull/226#issuecomment-3032799853

Testing done

Submitter checklist

  • [x] Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • [x] Ensure that the pull request title represents the desired changelog entry
  • [x] Please describe what you did
  • [ ] Link to relevant issues in GitHub or Jira
  • [ ] Link to relevant pull requests, esp. upstream and downstream changes
  • [x] Ensure you have provided tests that demonstrate the feature works or the issue is fixed

jgreffe avatar Jun 30 '25 16:06 jgreffe