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

Sentry external config not loading sentry.properties from disk automatically #1046

Closed
7 of 27 tasks
tlf30 opened this issue Nov 16, 2020 · 6 comments · Fixed by #1053
Closed
7 of 27 tasks

Sentry external config not loading sentry.properties from disk automatically #1046

tlf30 opened this issue Nov 16, 2020 · 6 comments · Fixed by #1053
Assignees
Labels
documentation enhancement New feature or request

Comments

@tlf30
Copy link

tlf30 commented Nov 16, 2020

Platform:

  • Android -> If yes, which Device API (and compileSdkVersion/targetSdkVersion/Build tools) version?
  • Java -> If yes, which Java (and sourceCompatibility/targetCompatibility) version?
    openjdk version "11.0.2" 2019-01-15
  • Kotlin -> If yes, which Kotlin (and jvmTarget) version?
  • NDK -> If yes, which NDK/CMake version?
  • React-Native -> If yes, which version?
  • Timber -> If yes, which version?
  • Log4j2 -> If yes, which version?
  • Logback -> If yes, which version?
  • Spring -> If yes, which version?

IDE:

  • Android Studio -> If yes, which version?
  • IntelliJ -> If yes, which version?
    IntelliJ IDEA 2020.2.3 (Ultimate Edition)
  • Other -> If yes, which one?

Build system:

  • Gradle -> If yes, which version?
    6.7
  • Buck -> If yes, which version?
  • Bazel -> If yes, which version?
  • Maven -> If yes, which version?
  • Other -> If yes, which one?

Android Gradle Plugin:

  • Yes -> If yes, which version?
  • No

Sentry Android Gradle Plugin:

  • Yes -> If yes, which version?
  • No

Proguard/R8:

  • Enabled
  • Disabled

Platform installed with:

  • JCenter
  • Bintray
  • Maven Central
  • Manually

The version of the SDK:
3.1.3


I have the following issue:

Sentry does not load sentry.properties from disk by default if the file is in the root directory with the application.
Adding the system property to tell sentry where the file is located is required to load the properties file.

        System.setProperty("sentry.properties.file", "sentry.properties");
        Sentry.init(options -> {
            options.setEnableExternalConfiguration(true);
        });

Steps to reproduce:
Place properties file in working directory with application when using external configuration.

Actual result:

  • Actual
    File is not loaded and no dsn is available.
Exception in thread "main" java.lang.IllegalArgumentException: DSN is required. Use empty string to disable SDK.
	at io.sentry.Sentry.initConfigurations(Sentry.java:183)
	at io.sentry.Sentry.init(Sentry.java:153)
	at io.sentry.Sentry.init(Sentry.java:124)
	at io.sentry.Sentry.init(Sentry.java:110)

Expected result:
Works as before with version 1 and auto detects the file.

@marandaneto
Copy link
Contributor

thanks for raising this.

I think it tries to read by default from the ClassLoader, cc @maciejwalkowiak to confirm.

@maciejwalkowiak
Copy link
Contributor

As @marandaneto said, by default it tries to load a file from the root of the classpath. Now I see that in 1.7.x in addition to that it did look into the root directory. I'll prepare a fix for that.

@marandaneto
Copy link
Contributor

not sure if it's documented already, but it'd be nice to document which configuration has precedence over others eg file from classpath, then file in the root, system envs, etc... otherwise, they will overwrite each other and it'll be hard to find out.

@maciejwalkowiak
Copy link
Contributor

It is in Javadocs: https://getsentry.github.io/sentry-java/io/sentry/config/PropertiesProviderFactory.html#create-- but not in the reference docs. I'll add there too.

@marandaneto
Copy link
Contributor

yeah worth adding it, this question also came thru different channels, thanks.

@marandaneto
Copy link
Contributor

Gonna be released on the next version :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants