Add console application sample. #499

maciejwalkowiak · 2020-07-29T21:49:34Z

📢 Type of change

Bugfix
New feature
Enhancement
Refactoring

📜 Description

Added console application sample. Code is a direct translation from console sample in sentry-dotnet repository excluding features that are not implemented in sentry-java.

Gradle contains minimal set of configuration to avoid confusion what's required to use Sentry. The only extra task that is added is one that builds fat jar - it's a subject to discussion if we need it here.

💡 Motivation and Context

Sample console application is a living documentation and guides users through Java SDK features.

💚 How did you test it?

📝 Checklist

I reviewed submitted code
I added tests to verify changes
All tests passing
No breaking changes

🔮 Next steps

Sample code is a direct translation from console sample in `sentry-dotnet` repository excluding features that are not implemented in `sentry-java`.

bruno-garcia

This is really promising! Thanks @maciejwalkowiak
Let's wait on @marandaneto's feedback, meanwhile I left some comments.

I'd also suggest we merge this into master. What do you think?
It's just a sample project and helps show off our API. Besides pushScope everything behaves the same in Android.

sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java

bruno-garcia · 2020-07-29T21:57:34Z

sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java

+      // The SDK is able to detect duplicate events:
+      // This is useful, for example, when multiple loggers log the same exception. Or exception is
+      // re-thrown and recaptured.
+      Sentry.captureException(exception);


Not sure we have this de-duplication here though, need to check

You're right, the deduplication does not happen.

yep, it's a TODO

bruno-garcia · 2020-07-29T21:58:24Z

sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java

+
+    // Console output will show queue being flushed. Task completes then and timeout is never
+    // reached (you don't need to wait a day :)
+    Sentry.flush(10000);


Not sure this will work. It should but we need to work on flush

You're right - it doesn't. SentryClient#flush is literally // TODO: Flush transport :-)

yep, I'd rather keep flush out of this sample, but change it to close which is already implemented?

Since close is executed anyway at the JVM termination I am not sure if this wouldn't be confusing. I'll leave it up to you and @bruno-garcia. This is anyway not the final version of the sample as it will get likely updated once more features are added to Java SDK.

that's true, if the integration call closes automatically by default, no need, or, to still sample it, but add as a comment that a default integration will do it, but they still could do it manually. @bruno-garcia ?

Addressed in #502.

sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java

Co-authored-by: Bruno Garcia <[email protected]>

marandaneto · 2020-07-30T07:33:56Z

sentry-samples/sentry-samples-console/build.gradle.kts

+val fatJar = task("fatJar", type = Jar::class) {
+    archiveFileName.set("${project.name}-fat.jar")
+    manifest {
+        attributes["Main-Class"] = "io.sentry.samples.console.Main"
+    }
+    from(configurations.runtimeClasspath.get().map { if (it.isDirectory) it else zipTree(it) })
+    with(tasks.jar.get() as CopySpec)
+}
+
+tasks {
+    "build" {
+        dependsOn(fatJar)
+    }
+}


nothing against doing it, but what's about sparing some bits and using a Gradle plugin? this might break when upgrading Gradle versions and usually plugins react fast to it.
eg https://github.com/johnrengelman/shadow
Also, is it really common that people copy paste fatjars instead of using maven dependencies?

I wanted to avoid extra dependency that's why I don't go with plugin.

If people build standalone CLI apps then it's common to build fat jars. It only depends if we want this example to show the code or also to actually let users run the sample application.

I believe both, but if they run, it'll be thru the IDE anyway, we don't export/allow to download the final jars/apks, it might be a good idea, but for that, we also have our official examples repo.
https://github.com/getsentry/examples

the idea of having samples on this repo is to easier the development, so you can run and debug the SDK and samples easily.

of course also a resource of learning.

that said, I'd go either with the plugin, so we don't need to maintain this piece of code or to not generate a fat jar at all.
thoughts @bruno-garcia ?

I am getting convinced that we can just remove fat jar building. It can be always added later if we find there is a need for that.

I guess removing then is best, otherwise tbh I'd also avoid the dependency and just leave those lines of code.
If gradle changed and broken I'm sure the plugin would take more time to catch up than us just patching the fix here.

I guess it boils down to how much pain have you had because of dependencies in your life. I avoid them as much as possible

@bruno-garcia gradle plugins are not dependencies

marandaneto · 2020-07-30T07:37:11Z

sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java

+          options.setDiagnosticLevel(SentryLevel.ERROR); //  A good option to have SDK debug log in prod is to use only level ERROR here.
+
+          // Using a proxy:
+          options.setProxy(null); // new Proxy(Type.HTTP, new InetSocketAddress(8090)));


I've never tested this class, to be honest, it's pretty much a copy-paste from sentry-java, it's fine to keep here but recently @thinkocapo could not use it for some reason and I still need to figure out if it's a bug or a proxy setup.

Removed in #502 as I believe we should have only working and tested code in sample. We can add it once we gain more confidence.

marandaneto

left a few comments/questions but pretty happy with it.
thanks @maciejwalkowiak

marandaneto · 2020-07-30T07:42:00Z

I'd also suggest we merge this into master. What do you think?

it'd be fine to go into master as this has no breaking changes, just a sample, although #500 should be merged into master too and I'm fine with it, let's do it

marandaneto · 2020-07-30T07:46:54Z

sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java

+
+  public static void main(String[] args) {
+    Sentry.init(
+        options -> {


I'd also set:

options.setRelease("[email protected]+1"); options.setSentryClientName("sentry-java/3.0.0");

while we don't read this automatically from the build conf/CI etc...so if people see this example that will land in master, they at least know that this is needed right now.

I rather ppl didn't know this existed. We'll start receiving all sorts of interesting values on the server if ppl are pushed to add something here.

maciejwalkowiak · 2020-07-30T08:26:39Z

Once @marandaneto or @bruno-garcia merge #501 I'll add all the polishing to this PR.

maciejwalkowiak · 2020-07-30T09:32:40Z

Closing in favour of #502.

Add console application sample.

b47adc4

Sample code is a direct translation from console sample in `sentry-dotnet` repository excluding features that are not implemented in `sentry-java`.

maciejwalkowiak requested review from bruno-garcia and marandaneto as code owners July 29, 2020 21:49

bruno-garcia reviewed Jul 29, 2020

View reviewed changes

Apply suggestions from code review

7dbc036

Co-authored-by: Bruno Garcia <[email protected]>

maciejwalkowiak mentioned this pull request Jul 29, 2020

Add method to add breadcrumb with string parameter. #500

Closed

8 tasks

Remove deduplication and flushing sample as they do not work.

527234d

marandaneto reviewed Jul 30, 2020

View reviewed changes

maciejwalkowiak mentioned this pull request Jul 30, 2020

Add method to add breadcrumb with string parameter. #501

Merged

8 tasks

maciejwalkowiak mentioned this pull request Jul 30, 2020

Add console application sample. #502

Merged

8 tasks

maciejwalkowiak closed this Jul 30, 2020

maciejwalkowiak deleted the samples-console branch July 30, 2020 09:32

Uh oh!

Add console application sample. #499

Add console application sample. #499

Uh oh!

Conversation

maciejwalkowiak commented Jul 29, 2020

📢 Type of change

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

Uh oh!

bruno-garcia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marandaneto Jul 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marandaneto left a comment

Choose a reason for hiding this comment

Uh oh!

marandaneto commented Jul 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maciejwalkowiak commented Jul 30, 2020

Uh oh!

maciejwalkowiak commented Jul 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marandaneto Jul 30, 2020 •

edited

Loading