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

Catch exceptions in flush timer and write to EventSource #696

Conversation

Illia-M
Copy link
Contributor

@Illia-M Illia-M commented May 23, 2020

Fixes #695 .

Changes

In JaegerUdpBatcher on flush timer tick exceptions in async void method cause application crash due to unhandled exception.
Add JaegerExporterEventSource and log handled exceptions on flush try from timer tick.

Checklist

  • I ran Unit Tests locally.

For significant contributions please make sure you have completed the following items:

  • Design discussion issue #
  • Changes in public API reviewed

The PR will automatically request reviews from code owners and trigger CI build and tests.

@cijothomas
Copy link
Member

@Illia-M You'll need to sign the CLA as this is your first time!
Thanks a lot for making the fix!

@Illia-M
Copy link
Contributor Author

Illia-M commented May 23, 2020

ou'll need to sign the CLA as this is your first time!
Thanks a lot for making the fix!

In progress, but it can take a while...

@CodeBlanch
Copy link
Member

LGTM

Copy link
Member

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Illia-M. Looks good, just one suggestion to move the try/catch inside FlushAsyncInternal.

@MikeGoldsmith
Copy link
Member

Thanks @Illia-M - LGTM.

@MikeGoldsmith
Copy link
Member

Looks like you'll need to update your PR with recent master branch changes before we can merge.

@Illia-M
Copy link
Contributor Author

Illia-M commented May 25, 2020

Looks like you'll need to update your PR with recent master branch changes before we can merge.

Updated

@MikeGoldsmith MikeGoldsmith merged commit 4aefc62 into open-telemetry:master May 25, 2020
MikeGoldsmith added a commit that referenced this pull request May 27, 2020
…ry/opentelemetry-dotnet into batching-activity-processor

* 'batching-activity-processor' of github.com:open-telemetry/opentelemetry-dotnet:
  Catch exceptions in flush timer and write to EventSource (#696)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JaegerUdpBatcher crash application when Jaeger down
4 participants