Skip to content
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

Unable to access the original Runnable object in SpringSchedulingRunnableWrapper #8661

Closed
kes2464 opened this issue Jun 7, 2023 Discussed in #8657 · 6 comments · Fixed by #8676
Closed

Unable to access the original Runnable object in SpringSchedulingRunnableWrapper #8661

kes2464 opened this issue Jun 7, 2023 Discussed in #8657 · 6 comments · Fixed by #8676
Labels
bug Something isn't working

Comments

@kes2464
Copy link

kes2464 commented Jun 7, 2023

Opening a bug. Could this be looked into?

Thanks

Discussed in #8657

Originally posted by kes2464 June 6, 2023
Hi

if (scheduledTask instanceof ScheduledMethodRunnable) {
  // do something
}

One of my applications have the above code which checks a type of Runnable instance and does something.
It's scheduled through Spring Task scheduler, so it's auto-instrumented.

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onSchedule(@Advice.Argument(value = 0, readOnly = false) Runnable runnable) {
if (SpringSchedulingTaskTracing.wrappingEnabled()) {
runnable = SpringSchedulingRunnableWrapper.wrapIfNeeded(runnable);
}
}

However, TaskSchedulerInstrumentation wraps the object and replaces the argument, so the condition is never true.

There's also no getter of the original instance in the wrapper.

Is there any common practice/workaround that people use apart from using reflection?

@mateuszrzeszutek mateuszrzeszutek added the bug Something isn't working label Jun 7, 2023
@laurit
Copy link
Contributor

laurit commented Jun 7, 2023

@kes2464 could you elaborate how do you observe our wrapped runnable or provide a sample app that reproduces the issue.

@kes2464
Copy link
Author

kes2464 commented Jun 8, 2023

Hi @laurit,

I have my own implementation of TaskScheduler which implements schedule methods.
Within the schedule methods it checks the type of the task runnable and do something.

So it's something like this

@Override
public ScheduledFuture<?> schedule(Runnable task, Trigger trigger) {
  if (task instanceof ScheduledMethodRunnable) {
    // do something
  }
}

Basically, when it’s auto-instrumented the Runnable is wrapped to SpringSchedulingRunnableWrapper before entering the method, it never goes into the if block.

@laurit
Copy link
Contributor

laurit commented Jun 8, 2023

@kes2464 then it should be resolved with #8676

@kes2464
Copy link
Author

kes2464 commented Jun 8, 2023

@kes2464 then it should be resolved with #8676

@laurit If I understood the change correctly, my implementation of TaskScheduler won’t be auto-instrumented and I probably need to add manual instrumentations, is that correct?

@laurit
Copy link
Contributor

laurit commented Jun 9, 2023

@kes2464 if your scheduler delegates to another scheduler that is one of the instrumented ones then that scheduler will produce spans. If not, then yes, you'll need to manually instrument it.

@kes2464
Copy link
Author

kes2464 commented Jun 10, 2023

@kes2464 if your scheduler delegates to another scheduler that is one of the instrumented ones then that scheduler will produce spans. If not, then yes, you'll need to manually instrument it.

Sounds good, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants