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

(WIP) feat: sentry-java beam integration #741

Open
wants to merge 9 commits into
base: master
from

Conversation

@Zylphrex
Copy link

commented Jul 25, 2019

No description provided.

@Zylphrex Zylphrex force-pushed the sentry-beam branch from f6f9261 to daead6a Jul 29, 2019

@Zylphrex Zylphrex requested a review from bruno-garcia Jul 30, 2019

@@ -0,0 +1,3 @@
# sentry-beam

See the [Sentry documentation](https://docs.sentry.io/clients/java/modules/beam/) for more information.

This comment has been minimized.

Copy link
@Zylphrex

Zylphrex Jul 30, 2019

Author

This doesn't exist yet


}

public static void capture(Event event) {

This comment has been minimized.

Copy link
@bruno-garcia

bruno-garcia Jul 31, 2019

Member

Do we want to expose a capture method here?
This might be confusing for docs etc when we have multiple entry points to the API.
Perhaps the way to go could be a client factory.

Like: https://github.com/getsentry/sentry-java/blob/master/sentry-appengine/src/main/java/io/sentry/appengine/AppEngineSentryClientFactory.java#L36

This comment has been minimized.

Copy link
@Zylphrex

Zylphrex Jul 31, 2019

Author

I had initially started with a client factory, see https://github.com/getsentry/sentry-java/pull/741/files/2ec5244751560bbc484cebe048db84aeabcb203c. However that my issue what the client factory was that it required me to pass it the variables like timestamp, boundedWindow, etc. at initialization time. For example, the end user code would look like

SentryClientFactory clientFactory = BeamSentryClientFactory.Builder.withTimestamp(timestamp)
        .withBoundedWindow(boundedWindow)
        // more
        .build();
Sentry.init("your dsn", clientFactory);

try {
    // beam code
} catch (Exception e) {
    Sentry.capture(e);
}

vs.

Sentry.init("your dsn");

try {
   // beam code
} catch (Exception e) {
    SentryBeam.withTimestamp(timestamp)
             .withBoundedWindow(boundedWindow)
             // more
             .capture(e);
}

I thought this implementation makes more sense as the first attempt makes it so that every event would have the same tags attached. Which might be okay in some distributed cases, but when running locally, it would overwrite the original sentry client and attach the wrong information onto the event. If there was a way to guarantee it's using a different sentry client, that may be better.

@Zylphrex

This comment has been minimized.

Copy link
Author

commented Aug 9, 2019

Seeing as how getsentry/sentry-python#446 was merged and utilizes python pickling, this should also utilize serialization to have uniformity across the SDKs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.