diff --git a/CHANGELOG.md b/CHANGELOG.md index 8eb9714ae02..b37311c4e04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * Enhancement: Set transaction name on events and transactions sent using Spring integration (#1067) * Fix: Set current thread only if there are no exceptions +* Enhancement: Set global tags on SentryOptions and load them from external configuration (#1066) # 4.0.0-alpha.1 diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index 62772bfb60b..7930da5ba96 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -105,7 +105,9 @@ class SentryAutoConfigurationTest { "sentry.proxy.port=8090", "sentry.proxy.user=proxy-user", "sentry.proxy.pass=proxy-pass", - "sentry.enable-tracing=true" + "sentry.enable-tracing=true", + "sentry.tags.tag1=tag1-value", + "sentry.tags.tag2=tag2-value" ).run { val options = it.getBean(SentryProperties::class.java) assertThat(options.readTimeoutMillis).isEqualTo(10) @@ -130,6 +132,7 @@ class SentryAutoConfigurationTest { assertThat(options.proxy!!.user).isEqualTo("proxy-user") assertThat(options.proxy!!.pass).isEqualTo("proxy-pass") assertThat(options.isEnableTracing).isTrue() + assertThat(options.tags).containsEntry("tag1", "tag1-value").containsEntry("tag2", "tag2-value") } } diff --git a/sentry/build.gradle.kts b/sentry/build.gradle.kts index 1ba24648340..122fbff377d 100644 --- a/sentry/build.gradle.kts +++ b/sentry/build.gradle.kts @@ -63,6 +63,8 @@ tasks { } test { environment["SENTRY_TEST_PROPERTY"] = "\"some-value\"" + environment["SENTRY_TEST_MAP_KEY1"] = "\"value1\"" + environment["SENTRY_TEST_MAP_KEY2"] = "value2" } } diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index 0a8fd749c5d..c260c67301c 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -5,6 +5,7 @@ import io.sentry.util.Objects; import java.util.ArrayList; import java.util.List; +import java.util.Map; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -89,6 +90,12 @@ private void processNonCachedEvent(final @NotNull SentryEvent event) { event.setSdk(options.getSdkVersion()); } + for (final Map.Entry tag : options.getTags().entrySet()) { + if (event.getTag(tag.getKey()) == null) { + event.setTag(tag.getKey(), tag.getValue()); + } + } + if (event.getThreads() == null) { // collecting threadIds that came from the exception mechanism, so we can mark threads as // crashed properly diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 510551b3c75..dd46507515f 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -11,7 +11,10 @@ import io.sentry.transport.NoOpTransportGate; import java.io.File; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLSocketFactory; @@ -240,6 +243,9 @@ public class SentryOptions { */ private boolean enableExternalConfiguration; + /** Tags applied to every event and transaction */ + private final @NotNull Map tags = new ConcurrentHashMap<>(); + /** * Creates {@link SentryOptions} from properties provided by a {@link PropertiesProvider}. * @@ -253,6 +259,10 @@ public class SentryOptions { options.setRelease(propertiesProvider.getProperty("release")); options.setDist(propertiesProvider.getProperty("dist")); options.setServerName(propertiesProvider.getProperty("servername")); + final Map tags = propertiesProvider.getMap("tags"); + for (final Map.Entry tag : tags.entrySet()) { + options.setTag(tag.getKey(), tag.getValue()); + } final String proxyHost = propertiesProvider.getProperty("proxy.host"); final String proxyUser = propertiesProvider.getProperty("proxy.user"); @@ -1118,6 +1128,25 @@ public void setEnableExternalConfiguration(boolean enableExternalConfiguration) this.enableExternalConfiguration = enableExternalConfiguration; } + /** + * Returns tags applied to all events and transactions. + * + * @return the tags map + */ + public @NotNull Map getTags() { + return tags; + } + + /** + * Sets a tag that is applied to all events and transactions. + * + * @param key the key + * @param value the value + */ + public void setTag(final @NotNull String key, final @NotNull String value) { + this.tags.put(key, value); + } + /** The BeforeSend callback */ public interface BeforeSendCallback { @@ -1203,6 +1232,10 @@ void merge(final @NotNull SentryOptions options) { if (options.getProxy() != null) { setProxy(options.getProxy()); } + final Map tags = new HashMap<>(options.getTags()); + for (final Map.Entry tag : tags.entrySet()) { + this.tags.put(tag.getKey(), tag.getValue()); + } } private @NotNull SdkVersion createSdkVersion() { diff --git a/sentry/src/main/java/io/sentry/config/AbstractPropertiesProvider.java b/sentry/src/main/java/io/sentry/config/AbstractPropertiesProvider.java new file mode 100644 index 00000000000..5f0f15e2b5c --- /dev/null +++ b/sentry/src/main/java/io/sentry/config/AbstractPropertiesProvider.java @@ -0,0 +1,58 @@ +package io.sentry.config; + +import io.sentry.util.Objects; +import io.sentry.util.StringUtils; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Base class for properties provider resolving properties from {@link Properties} based sources. + */ +abstract class AbstractPropertiesProvider implements PropertiesProvider { + private final @NotNull String prefix; + private final @NotNull Properties properties; + + protected AbstractPropertiesProvider( + final @NotNull String prefix, final @NotNull Properties properties) { + this.prefix = Objects.requireNonNull(prefix, "prefix is required"); + this.properties = Objects.requireNonNull(properties, "properties are required"); + } + + protected AbstractPropertiesProvider(final @NotNull Properties properties) { + this("", properties); + } + + @Override + public @Nullable String getProperty(final @NotNull String property) { + return StringUtils.removeSurrounding(properties.getProperty(prefix + property), "\""); + } + + /** + * Gets property map based on the property name, where property name is a prefix for the expected + * property name. For example, when {@link #prefix} is set to {@code "sentry."} and following + * properties are set: sentry.tags.tag1=value1 and sentry.tags.tag2=value2, calling {@code + * getMap("tags")} returns a map containing pairs "tag1" => "value1" and "tag2" => "value2". + * + * @param property the property name + * @return the map or empty if no matching environment variables found. + */ + @Override + public @NotNull Map getMap(final @NotNull String property) { + final String prefix = this.prefix + property + "."; + + final Map result = new HashMap<>(); + for (Map.Entry entry : properties.entrySet()) { + if (entry.getKey() instanceof String && entry.getValue() instanceof String) { + final String key = (String) entry.getKey(); + if (key.startsWith(prefix)) { + final String value = StringUtils.removeSurrounding((String) entry.getValue(), "\""); + result.put(key.substring(prefix.length()), value); + } + } + } + return result; + } +} diff --git a/sentry/src/main/java/io/sentry/config/CompositePropertiesProvider.java b/sentry/src/main/java/io/sentry/config/CompositePropertiesProvider.java index 851801f1f37..3414c62a53a 100644 --- a/sentry/src/main/java/io/sentry/config/CompositePropertiesProvider.java +++ b/sentry/src/main/java/io/sentry/config/CompositePropertiesProvider.java @@ -1,6 +1,8 @@ package io.sentry.config; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -26,4 +28,15 @@ public CompositePropertiesProvider(@NotNull List providers) } return null; } + + @Override + public @NotNull Map getMap(final @NotNull String property) { + for (final PropertiesProvider provider : providers) { + final Map result = provider.getMap(property); + if (!result.isEmpty()) { + return result; + } + } + return new ConcurrentHashMap<>(); + } } diff --git a/sentry/src/main/java/io/sentry/config/EnvironmentVariablePropertiesProvider.java b/sentry/src/main/java/io/sentry/config/EnvironmentVariablePropertiesProvider.java index 41ed662b6e8..8a8680cc64c 100644 --- a/sentry/src/main/java/io/sentry/config/EnvironmentVariablePropertiesProvider.java +++ b/sentry/src/main/java/io/sentry/config/EnvironmentVariablePropertiesProvider.java @@ -2,6 +2,8 @@ import io.sentry.util.StringUtils; import java.util.Locale; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -15,6 +17,34 @@ final class EnvironmentVariablePropertiesProvider implements PropertiesProvider @Override public @Nullable String getProperty(@NotNull String property) { return StringUtils.removeSurrounding( - System.getenv(PREFIX + "_" + property.replace(".", "_").toUpperCase(Locale.ROOT)), "\""); + System.getenv(propertyToEnvironmentVariableName(property)), "\""); + } + + /** + * Gets property map based on the property name, where property name is a prefix for the expected + * environment variable name. For example, when following environment variables are set: + * SENTRY_TAGS_TAG_1=value1 and SENTRY_TAGS_TAG_2=value2, calling {@code getMap("tags")} returns a + * map containing pairs "tag1" => "value1" and "tag2" => "value2". + * + * @param property the property name + * @return the map or empty if no matching environment variables found. + */ + @Override + public @NotNull Map getMap(final @NotNull String property) { + final String prefix = propertyToEnvironmentVariableName(property) + "_"; + + final Map result = new ConcurrentHashMap<>(); + for (Map.Entry entry : System.getenv().entrySet()) { + final String key = entry.getKey(); + if (key.startsWith(prefix)) { + final String value = StringUtils.removeSurrounding(entry.getValue(), "\""); + result.put(key.substring(prefix.length()).toLowerCase(Locale.ROOT), value); + } + } + return result; + } + + private @NotNull String propertyToEnvironmentVariableName(final @NotNull String property) { + return PREFIX + "_" + property.replace(".", "_").toUpperCase(Locale.ROOT); } } diff --git a/sentry/src/main/java/io/sentry/config/PropertiesProvider.java b/sentry/src/main/java/io/sentry/config/PropertiesProvider.java index 080a6772be4..3ddc95be030 100644 --- a/sentry/src/main/java/io/sentry/config/PropertiesProvider.java +++ b/sentry/src/main/java/io/sentry/config/PropertiesProvider.java @@ -1,5 +1,6 @@ package io.sentry.config; +import java.util.Map; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -13,6 +14,15 @@ public interface PropertiesProvider { @Nullable String getProperty(@NotNull String property); + /** + * Resolves a map for a property given by it's name. + * + * @param property - the property name + * @return the map or empty map if not found + */ + @NotNull + Map getMap(final @NotNull String property); + /** * Resolves property given by it's name. * diff --git a/sentry/src/main/java/io/sentry/config/SimplePropertiesProvider.java b/sentry/src/main/java/io/sentry/config/SimplePropertiesProvider.java index c9fdd2935c1..af80c725e9e 100644 --- a/sentry/src/main/java/io/sentry/config/SimplePropertiesProvider.java +++ b/sentry/src/main/java/io/sentry/config/SimplePropertiesProvider.java @@ -1,20 +1,12 @@ package io.sentry.config; -import io.sentry.util.StringUtils; import java.util.Properties; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; /** {@link PropertiesProvider} implementation that delegates to {@link Properties}. */ -final class SimplePropertiesProvider implements PropertiesProvider { - private final @NotNull Properties properties; +final class SimplePropertiesProvider extends AbstractPropertiesProvider { public SimplePropertiesProvider(final @NotNull Properties properties) { - this.properties = properties; - } - - @Override - public @Nullable String getProperty(@NotNull String property) { - return StringUtils.removeSurrounding(properties.getProperty(property), "\""); + super(properties); } } diff --git a/sentry/src/main/java/io/sentry/config/SystemPropertyPropertiesProvider.java b/sentry/src/main/java/io/sentry/config/SystemPropertyPropertiesProvider.java index fcd81d52654..96d970531fe 100644 --- a/sentry/src/main/java/io/sentry/config/SystemPropertyPropertiesProvider.java +++ b/sentry/src/main/java/io/sentry/config/SystemPropertyPropertiesProvider.java @@ -1,17 +1,12 @@ package io.sentry.config; -import io.sentry.util.StringUtils; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - /** * {@link PropertiesProvider} implementation that resolves properties from Java system properties. */ -final class SystemPropertyPropertiesProvider implements PropertiesProvider { - private static final String PREFIX = "sentry"; +final class SystemPropertyPropertiesProvider extends AbstractPropertiesProvider { + private static final String PREFIX = "sentry."; - @Override - public @Nullable String getProperty(@NotNull String property) { - return StringUtils.removeSurrounding(System.getProperty(PREFIX + "." + property), "\""); + public SystemPropertyPropertiesProvider() { + super(PREFIX, System.getProperties()); } } diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index c5ce69c82c0..5481d482aa1 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -25,10 +25,11 @@ class MainEventProcessorTest { version = "1.2.3" } } - fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true, environment: String? = "environment"): MainEventProcessor { + fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true, environment: String? = "environment", tags: Map = emptyMap()): MainEventProcessor { sentryOptions.isAttachThreads = attachThreads sentryOptions.isAttachStacktrace = attachStackTrace sentryOptions.environment = environment + tags.forEach { sentryOptions.setTag(it.key, it.value) } return MainEventProcessor(sentryOptions) } } @@ -207,6 +208,25 @@ class MainEventProcessorTest { assertEquals("custom", event.environment) } + @Test + fun `sets tags from SentryOptions`() { + val sut = fixture.getSut(tags = mapOf("tag1" to "value1", "tag2" to "value2")) + val event = SentryEvent() + sut.process(event, null) + assertEquals("value1", event.tags["tag1"]) + assertEquals("value2", event.tags["tag2"]) + } + + @Test + fun `when event has a tag set with the same name as SentryOptions tags, the tag value from the event is retained`() { + val sut = fixture.getSut(tags = mapOf("tag1" to "value1", "tag2" to "value2")) + val event = SentryEvent() + event.setTag("tag2", "event-tag-value") + sut.process(event, null) + assertEquals("value1", event.tags["tag1"]) + assertEquals("event-tag-value", event.tags["tag2"]) + } + private fun generateCrashedEvent(crashedThread: Thread = Thread.currentThread()) = SentryEvent().apply { val mockThrowable = mock() val actualThrowable = UncaughtExceptionHandlerIntegration.getUnhandledThrowable(crashedThread, mockThrowable) diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index c67a4601924..4f472b67715 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -173,6 +173,8 @@ class SentryOptionsTest { externalOptions.release = "release" externalOptions.serverName = "serverName" externalOptions.proxy = SentryOptions.Proxy("example.com", "8090") + externalOptions.setTag("tag1", "value1") + externalOptions.setTag("tag2", "value2") val options = SentryOptions() options.merge(externalOptions) @@ -185,6 +187,21 @@ class SentryOptionsTest { assertNotNull(options.proxy) assertEquals("example.com", options.proxy!!.host) assertEquals("8090", options.proxy!!.port) + assertEquals(mapOf("tag1" to "value1", "tag2" to "value2"), options.tags) + } + + @Test + fun `merging options merges and overwrites existing tag values`() { + val externalOptions = SentryOptions() + externalOptions.setTag("tag1", "value1") + externalOptions.setTag("tag2", "value2") + val options = SentryOptions() + options.setTag("tag2", "original-options-value") + options.setTag("tag3", "value3") + + options.merge(externalOptions) + + assertEquals(mapOf("tag1" to "value1", "tag2" to "value2", "tag3" to "value3"), options.tags) } @Test @@ -231,4 +248,23 @@ class SentryOptionsTest { temporaryFolder.delete() } } + + @Test + fun `creates options with tags using external properties`() { + // create a sentry.properties file in temporary folder + val temporaryFolder = TemporaryFolder() + temporaryFolder.create() + val file = temporaryFolder.newFile("sentry.properties") + file.appendText("tags.tag1=value1\n") + file.appendText("tags.tag2=value2") + // set location of the sentry.properties file + System.setProperty("sentry.properties.file", file.absolutePath) + + try { + val options = SentryOptions.from(PropertiesProviderFactory.create()) + assertEquals(mapOf("tag1" to "value1", "tag2" to "value2"), options.tags) + } finally { + temporaryFolder.delete() + } + } } diff --git a/sentry/src/test/java/io/sentry/config/EnvironmentVariablePropertiesProviderTest.kt b/sentry/src/test/java/io/sentry/config/EnvironmentVariablePropertiesProviderTest.kt index 83cda2c3f04..4b45217e93a 100644 --- a/sentry/src/test/java/io/sentry/config/EnvironmentVariablePropertiesProviderTest.kt +++ b/sentry/src/test/java/io/sentry/config/EnvironmentVariablePropertiesProviderTest.kt @@ -19,4 +19,11 @@ class EnvironmentVariablePropertiesProviderTest { val result = provider.getProperty("not.set.property") assertNull(result) } + + @Test + fun `resolves map properties`() { + // SENTRY_TEST_MAP_KEY1 and SENTRY_TEST_MAP_KEY2 are set in Gradle build configuration + val result = provider.getMap("test.map") + assertEquals(mapOf("key1" to "value1", "key2" to "value2"), result) + } } diff --git a/sentry/src/test/java/io/sentry/config/SimplePropertiesProviderTest.kt b/sentry/src/test/java/io/sentry/config/SimplePropertiesProviderTest.kt index 8601c574af9..f199e5a66f0 100644 --- a/sentry/src/test/java/io/sentry/config/SimplePropertiesProviderTest.kt +++ b/sentry/src/test/java/io/sentry/config/SimplePropertiesProviderTest.kt @@ -1,14 +1,14 @@ package io.sentry.config import java.util.Properties +import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNull -import org.junit.Test class SimplePropertiesProviderTest { @Test - fun `when system property is set resolves property`() { + fun `when property is set resolves property`() { val properties = Properties() properties["some-property"] = "\"some-value\"" val provider = SimplePropertiesProvider(properties) @@ -18,10 +18,21 @@ class SimplePropertiesProviderTest { } @Test - fun `when system property is not set returns null`() { + fun `when property is not set returns null`() { val provider = SimplePropertiesProvider(Properties()) val result = provider.getProperty("some-property") assertNull(result) } + + @Test + fun `resolves map properties`() { + val properties = Properties() + properties["some-property.key1"] = "\"value1\"" + properties["some-property.key2"] = "\"value2\"" + val provider = SimplePropertiesProvider(properties) + + val result = provider.getMap("some-property") + assertEquals(mapOf("key1" to "value1", "key2" to "value2"), result) + } } diff --git a/sentry/src/test/java/io/sentry/config/SystemPropertyPropertiesProviderTest.kt b/sentry/src/test/java/io/sentry/config/SystemPropertyPropertiesProviderTest.kt index 361bf35c7ff..c3ad9eb8d02 100644 --- a/sentry/src/test/java/io/sentry/config/SystemPropertyPropertiesProviderTest.kt +++ b/sentry/src/test/java/io/sentry/config/SystemPropertyPropertiesProviderTest.kt @@ -25,4 +25,13 @@ class SystemPropertyPropertiesProviderTest { val result = provider.getProperty("dsn") assertNull(result) } + + @Test + fun `resolves map from system properties`() { + System.setProperty("sentry.some-property.key1", "\"value1\"") + System.setProperty("sentry.some-property.key2", "\"value2\"") + + val result = provider.getMap("some-property") + assertEquals(mapOf("key1" to "value1", "key2" to "value2"), result) + } }