getsentry / sentry-java

A Sentry SDK for Java, Android and other JVM languages.
https://docs.sentry.io/
MIT License
1.16k stars 435 forks source link

Springboot and Logback Configuration not working as expected #2457

Closed maertu closed 1 year ago

maertu commented 1 year ago

Integration

sentry

Java Version

17.0.3

Version

6.11.0

Steps to Reproduce

logback-spring.config

<configuration>
    <appender name="Console" class="ch.qos.logback.core.ConsoleAppender">
        <encoder>
            <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
        </encoder>
    </appender>

    <appender name="Sentry" class="io.sentry.logback.SentryAppender">
        <minimumEventLevel>WARN</minimumEventLevel>
        <options>
            <dsn>https://somedsn@company.com/12</dsn>
        </options>
    </appender>
    <root level="info">
        <appender-ref ref="Sentry" />
        <appender-ref ref="Console"/>
    </root>
</configuration>

application.yml

sentry:
  enabled: true
  dsn: https://somedsn@company.com/12
  exception-resolver-order: -2147483647
  logging.minimum-event-level: warn
  release: @project.name@ @project.version@
  tags:
    zone: x2

pom

        <dependency>
            <groupId>io.sentry</groupId>
            <artifactId>sentry-spring-boot-starter</artifactId>
            <version>6.11.0</version>
        </dependency>
        <dependency>
            <groupId>io.sentry</groupId>
            <artifactId>sentry-logback</artifactId>
            <version>6.11.0</version>
        </dependency>

Expected Result

For logs on startup, when SpringBoot is not yet loaded:

For logs after startup:

Actual Result

Events logged after SpringBoot started, don't use the configuration of application.yml. However the instruction on this site : https://docs.sentry.io/platforms/java/guides/spring-boot/logging-frameworks/ states:

You do not need to configure your DSN in the Logback configuration file since Sentry is configured from the Spring Boot integration. However, if errors that may appear during startup should to be sent to Sentry, the DSN must be provided to both the Logback and Spring Boot configurations

So I expect the configuration is correct and I need to add the DSN to logback-spring-config and SpringBoot-Configuration. However as soon as defining logback-appender in logback-spring-config, SpringBoot Properties are no longer respected at all.

Could be an expected behaviour, because there is also this statement:

If you decide to opt-out from the application.properties based Spring Boot logging configuration, and instead configure logging in the logback-spring.xml file, the SentryAppender can be configured as follows: ...

However I don't see, how those two cites can coexist? First one states you should provide property in both places. Second statement states that when defined properties in logback-spring-config, you opt-out from SpringBoot-Configuration.

Do I get something wrong here?

Edit: The goal is to have the ability to use spring-boot applications.yml for configuring Sentry, to have access to all configuration properties, like "Tags" and "exception-resolver-order", while still beeing able to get logs on startup.

adinauer commented 1 year ago

Hello @maertu I just tested with our Spring Boot sample and it calls init twice. Once from the SentryAppender and once from SentryAutoConfiguration. I added a logback-spring.xml config file to the sample to test. Events created e.g. in a controller have the tag set while early log messages (during startup) do not. Seems to be working for me.

Can you please try enabling debug in your config (https://docs.sentry.io/platforms/java/guides/spring-boot/configuration/#debug)? Maybe this'll show why it's not working.

maertu commented 1 year ago

@adinauer Thanks for your investigations. I found out what's the problem. I defined the Sentry configuration for Spring in application.yml. However this not seem to work. Don't know if this is not working at all or just not working in conjunction with logback. I was able to solve the issue by moving the sentry configuration into sentry.properties file. Not perfect as it would be nice to have it at central point (application.yml) but good enough.

I don't know if this is intentional. If yes maybe it would be good to add a hint to https://docs.sentry.io/platforms/java/guides/spring-boot/

adinauer commented 1 year ago

@maertu I just retested with application.yml and that works too. Only problem I had was with release: @project.name@ @project.version@ failing to parse. I presume these are Maven placeholders that will be successfully replaced in your setup.

maertu commented 1 year ago

@adinauer Thanks for trying it out, yes those are Maven placeholders. Well that's strange. I verified the behavior again, and in my case none of the properties in application.yml are respected. Events are generated, but in the exact same style as they are before spring-boot, (no tags etc.) What I was unsure about anyway, was the "logging.minimum-event-level: warn". I guess this property is wrong or logback-only, as it's not listed here https://docs.sentry.io/platforms/java/guides/spring-boot/configuration/#options But even removing it does not fix it. You may share with what exact properties you tested? Or did you use exactly mine?

adinauer commented 1 year ago

Yes, minimum-event-level and minimum-breadcrumb-level are specific to logging integrations, not Spring (Boot).

Here's the application.yml I used to test, please note there's some you may not want (especially sendDefaultPii):

sentry:
  dsn: https://...
  sendDefaultPii: true
  maxRequestBodySize: medium
  maxBreadcrumbs: 150
  tracesSampleRate: 1.0
  debug: true
  exception-resolver-order: -2147483647
  logging.minimum-event-level: debug
  release: -project.name- -project.version-
  tags:
    zone: x2
spring:
  datasource:
    url: jdbc:p6spy:hsqldb:mem:testdb
    dirverClassName: com.p6spy.engine.spy.P6SpyDriver
    username: sa
    password:

And this is the logback-spring.xml I used:

<configuration>
    <appender name="Console" class="ch.qos.logback.core.ConsoleAppender">
        <encoder>
            <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
        </encoder>
    </appender>

    <appender name="Sentry" class="io.sentry.logback.SentryAppender">
        <minimumEventLevel>DEBUG</minimumEventLevel>
        <options>
            <dsn>https://...</dsn>
        </options>
    </appender>
    <root level="info">
        <appender-ref ref="Sentry" />
        <appender-ref ref="Console"/>
    </root>
</configuration>

You could try removing all of them and adding them back one by one. Maybe that helps.

maertu commented 1 year ago

I tried it with this config. But for me the behaviour is the same, events appear in sentry but no tags etc. I don't know if this may is related to SpringBoot 3.0. Anyway for me the issue is solved as i will just use sentry.properties and this works fine for now.

adinauer commented 1 year ago

@maertu just to confirm, you are using sentry-spring-boot-starter-jakarta with Spring Boot 3, correct? sentry-spring-boot-starter is meant for older versions of Spring Boot.

I just retested using our sentry-samples-spring-boot-jakarta sample and it too works as expected. I presume the problem you're experiencing comes from combining Spring Boot 3 with sentry-spring-boot-starter (note the missing jakarta in the name).

maertu commented 1 year ago

@adinauer Ahhh, that's it! I just followed the instruction for spring-boot config here link Was this the wrong page to lookup and there is a newer one regarding Spring-Boot 3.0? Or is it just not up-to-date? If latter, it may would be good to inlucde a hint to the new "*jakarta" dependency. Thanks anyway

adinauer commented 1 year ago

We haven't yet updated docs. Will update soon.

adinauer commented 1 year ago

We've just updated docs https://docs.sentry.io/platforms/java/guides/spring-boot/