workflow-step-api-plugin
workflow-step-api-plugin copied to clipboard
Add `SynchronousNonBlockingStepExecution.ExecutorServiceAugmentor` extension point
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
opentelemetrywants 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 thejunitstep.In order for this to work, plugins need access to an OTel
Contextin their build steps and/or related code so that they can produce aSpanwith the correct parent. By default, OTel contexts are stored as aThreadLocalvariable.opentelemetry's synchronousGraphListenerimplementation sets up this context on the CPS VM thread when a new step starts. For Pipeline steps with fully synchronousStepExecutions, that is enough for context propagation to work correctly within the extent ofStepExecution.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.
opentelemetryplugin currently handles this case by reflectively modifyingSynchronousNonBlockingStepExecution.executorServiceto wrap it withio.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 allowopentelemetryto augmentSynchronousNonBlockingStepExecution.executorServicewithContext.taskWrappingwithout 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-storagethis would mean that thejunitstep 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
Contextinto theStepContextfor each step automatically, and then have some kind of method likeOtelPlugin.useContext(StepContext)that sets up the thread local for plugins that need it? This would only work for PipelineSteps, but maybe that's ok?- There is a
ContextStorageProviderAPI. 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