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

RUM-3068 Add support for global attributes on logs #1900

Merged
merged 5 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions features/dd-sdk-android-logs/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ class com.datadog.android.log.Logger
object com.datadog.android.log.Logs
fun enable(LogsConfiguration, com.datadog.android.api.SdkCore = Datadog.getInstance())
fun isEnabled(com.datadog.android.api.SdkCore = Datadog.getInstance()): Boolean
fun addAttribute(String, Any?, com.datadog.android.api.SdkCore = Datadog.getInstance())
fun removeAttribute(String, com.datadog.android.api.SdkCore = Datadog.getInstance())
data class com.datadog.android.log.LogsConfiguration
class Builder
fun useCustomEndpoint(String): Builder
Expand Down
6 changes: 6 additions & 0 deletions features/dd-sdk-android-logs/api/dd-sdk-android-logs.api
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,18 @@ public final class com/datadog/android/log/Logger$Builder {

public final class com/datadog/android/log/Logs {
public static final field INSTANCE Lcom/datadog/android/log/Logs;
public static final fun addAttribute (Ljava/lang/String;Ljava/lang/Object;)V
public static final fun addAttribute (Ljava/lang/String;Ljava/lang/Object;Lcom/datadog/android/api/SdkCore;)V
public static synthetic fun addAttribute$default (Ljava/lang/String;Ljava/lang/Object;Lcom/datadog/android/api/SdkCore;ILjava/lang/Object;)V
public static final fun enable (Lcom/datadog/android/log/LogsConfiguration;)V
public static final fun enable (Lcom/datadog/android/log/LogsConfiguration;Lcom/datadog/android/api/SdkCore;)V
public static synthetic fun enable$default (Lcom/datadog/android/log/LogsConfiguration;Lcom/datadog/android/api/SdkCore;ILjava/lang/Object;)V
public static final fun isEnabled ()Z
public static final fun isEnabled (Lcom/datadog/android/api/SdkCore;)Z
public static synthetic fun isEnabled$default (Lcom/datadog/android/api/SdkCore;ILjava/lang/Object;)Z
public static final fun removeAttribute (Ljava/lang/String;)V
public static final fun removeAttribute (Ljava/lang/String;Lcom/datadog/android/api/SdkCore;)V
public static synthetic fun removeAttribute$default (Ljava/lang/String;Lcom/datadog/android/api/SdkCore;ILjava/lang/Object;)V
}

public final class com/datadog/android/log/LogsConfiguration {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package com.datadog.android.log

import com.datadog.android.Datadog
import com.datadog.android.api.InternalLogger
import com.datadog.android.api.SdkCore
import com.datadog.android.api.feature.Feature
import com.datadog.android.api.feature.FeatureSdkCore
Expand Down Expand Up @@ -51,4 +52,63 @@ object Logs {
fun isEnabled(sdkCore: SdkCore = Datadog.getInstance()): Boolean {
return (sdkCore as FeatureSdkCore).getFeature(Feature.LOGS_FEATURE_NAME) != null
}

/**
* Add a custom attribute to all future logs sent by loggers created from the given SDK core.
*
* Values can be nested up to 10 levels deep. Keys
* using more than 10 levels will be sanitized by SDK.
*
* @param key the key for this attribute
* @param value the attribute value
* @param sdkCore the [SdkCore] instance to add the attribute to. If not provided, the default
* instance is used.
*/
@JvmOverloads
@JvmStatic
fun addAttribute(key: String, value: Any?, sdkCore: SdkCore = Datadog.getInstance()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we accept Any? or just Any ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept it in line with Logger's interface which accepts Any?

val featureCore = sdkCore as FeatureSdkCore
val logsFeature = featureCore.getFeature(Feature.LOGS_FEATURE_NAME)?.unwrap<LogsFeature>()
if (logsFeature == null) {
sdkCore.internalLogger.log(
InternalLogger.Level.ERROR,
InternalLogger.Target.USER,
{ LOGS_NOT_ENABLED_MESSAGE }
)
return
} else {
logsFeature.addAttribute(key, value)
}
}

/**
* Remove a custom attribute from all future logs sent by loggers created from the given SDK core.
*
* Previous logs won't lose the attribute value associated with this key if they were created
* prior to this call.
*
* @param key the key of the attribute to remove
* @param sdkCore the [SdkCore] instance to remove the attribute from. If not provided, the default
* instance is used.
*/
@JvmOverloads
@JvmStatic
fun removeAttribute(key: String, sdkCore: SdkCore = Datadog.getInstance()) {
val featureCore = sdkCore as FeatureSdkCore
val logsFeature = featureCore.getFeature(Feature.LOGS_FEATURE_NAME)?.unwrap<LogsFeature>()
if (logsFeature == null) {
sdkCore.internalLogger.log(
InternalLogger.Level.ERROR,
InternalLogger.Target.USER,
{ LOGS_NOT_ENABLED_MESSAGE }
)
return
} else {
logsFeature.removeAttribute(key)
}
}

internal const val LOGS_NOT_ENABLED_MESSAGE =
"You're trying to add attributes to logs, but the feature is not enabled. " +
"Please enable it first."
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.datadog.android.api.net.RequestFactory
import com.datadog.android.api.storage.DataWriter
import com.datadog.android.api.storage.FeatureStorageConfiguration
import com.datadog.android.core.feature.event.JvmCrash
import com.datadog.android.core.internal.utils.NULL_MAP_VALUE
import com.datadog.android.event.EventMapper
import com.datadog.android.event.MapperSerializer
import com.datadog.android.log.internal.domain.DatadogLogGenerator
Expand All @@ -30,6 +31,7 @@ import com.datadog.android.log.internal.storage.LogsDataWriter
import com.datadog.android.log.internal.storage.NoOpDataWriter
import com.datadog.android.log.model.LogEvent
import java.util.Locale
import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicBoolean
Expand All @@ -47,6 +49,43 @@ internal class LogsFeature(
internal val initialized = AtomicBoolean(false)
internal var packageName = ""
private val logGenerator = DatadogLogGenerator()
private val attributes = ConcurrentHashMap<String, Any?>()
0xnm marked this conversation as resolved.
Show resolved Hide resolved

// region Context Information (attributes)
/**
* Add a custom attribute to all logs sent by any logger created from this feature.
*
* Values can be nested up to 10 levels deep. Keys
* using more than 10 levels will be sanitized by SDK.
*
* @param key the key for this attribute
* @param value the attribute value
*/
internal fun addAttribute(key: String, value: Any?) {
if (value == null) {
attributes[key] = NULL_MAP_VALUE
} else {
attributes[key] = value
}
}

/**
* Remove a custom attribute from all future logs sent by any logger created from this feature.
* Previous logs won't lose the attribute value associated with this key if they were created
* prior to this call.
* @param key the key of the attribute to remove
*/
internal fun removeAttribute(key: String) {
@Suppress("UnsafeThirdPartyFunctionCall") // NPE cannot happen here
attributes.remove(key)
}

internal fun getAttributes(): Map<String, Any?> {
@Suppress("UnsafeThirdPartyFunctionCall") // NPE cannot happen here
return attributes.toMap()
}
fuzzybinary marked this conversation as resolved.
Show resolved Hide resolved

// endregion

// region Feature

Expand Down Expand Up @@ -76,6 +115,8 @@ internal class LogsFeature(
dataWriter = NoOpDataWriter()
packageName = ""
initialized.set(false)
@Suppress("UnsafeThirdPartyFunctionCall")
attributes.clear()
}

// endregion
Expand Down Expand Up @@ -129,6 +170,7 @@ internal class LogsFeature(
@Suppress("UnsafeThirdPartyFunctionCall") // argument is good
val lock = CountDownLatch(1)

val attributes = getAttributes()
sdkCore.getFeature(name)
?.withWriteContext { datadogContext, eventBatchWriter ->
val log = logGenerator.generateLog(
Expand All @@ -138,7 +180,7 @@ internal class LogsFeature(
loggerName = jvmCrash.loggerName,
message = jvmCrash.message,
throwable = jvmCrash.throwable,
attributes = emptyMap(),
attributes = attributes,
timestamp = jvmCrash.timestamp,
bundleWithTraces = true,
bundleWithRum = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import com.datadog.android.api.feature.FeatureSdkCore
import com.datadog.android.api.storage.DataWriter
import com.datadog.android.core.sampling.RateBasedSampler
import com.datadog.android.core.sampling.Sampler
import com.datadog.android.log.internal.LogsFeature
import com.datadog.android.log.internal.domain.LogGenerator
import com.datadog.android.log.model.LogEvent
import android.util.Log as AndroidLog
Expand Down Expand Up @@ -44,8 +45,13 @@ internal class DatadogLogHandler(
}

val resolvedTimeStamp = timestamp ?: System.currentTimeMillis()
val combinedAttributes = mutableMapOf<String, Any?>()
val logsFeature = sdkCore.getFeature(Feature.LOGS_FEATURE_NAME)
if (logsFeature != null) {
combinedAttributes.putAll(logsFeature.unwrap<LogsFeature>().getAttributes().toMutableMap())
}
combinedAttributes.putAll(attributes)
if (sampler.sample()) {
val logsFeature = sdkCore.getFeature(Feature.LOGS_FEATURE_NAME)
if (logsFeature != null) {
val threadName = Thread.currentThread().name
logsFeature.withWriteContext { datadogContext, eventBatchWriter ->
Expand All @@ -54,7 +60,7 @@ internal class DatadogLogHandler(
datadogContext,
message,
throwable,
attributes,
combinedAttributes,
tags,
threadName,
resolvedTimeStamp
Expand All @@ -81,7 +87,7 @@ internal class DatadogLogHandler(
"type" to "logger_error",
"message" to message,
"throwable" to throwable,
"attributes" to attributes
"attributes" to combinedAttributes
)
)
} else {
Expand All @@ -94,6 +100,7 @@ internal class DatadogLogHandler(
}
}

@Suppress("LongMethod")
override fun handleLog(
level: Int,
message: String,
Expand All @@ -109,8 +116,14 @@ internal class DatadogLogHandler(
}

val resolvedTimeStamp = timestamp ?: System.currentTimeMillis()
val combinedAttributes = mutableMapOf<String, Any?>()
val logsFeature = sdkCore.getFeature(Feature.LOGS_FEATURE_NAME)
if (logsFeature != null) {
combinedAttributes.putAll(logsFeature.unwrap<LogsFeature>().getAttributes().toMutableMap())
}
combinedAttributes.putAll(attributes)

if (sampler.sample()) {
val logsFeature = sdkCore.getFeature(Feature.LOGS_FEATURE_NAME)
if (logsFeature != null) {
val threadName = Thread.currentThread().name
logsFeature.withWriteContext { datadogContext, eventBatchWriter ->
Expand All @@ -121,7 +134,7 @@ internal class DatadogLogHandler(
errorKind,
errorMessage,
errorStacktrace,
attributes,
combinedAttributes,
tags,
threadName,
resolvedTimeStamp
Expand All @@ -148,7 +161,7 @@ internal class DatadogLogHandler(
"type" to "logger_error_with_stacktrace",
"message" to message,
"stacktrace" to errorStacktrace,
"attributes" to attributes
"attributes" to combinedAttributes
)
)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@

package com.datadog.android.log

import com.datadog.android.api.InternalLogger
import com.datadog.android.api.feature.Feature
import com.datadog.android.api.feature.FeatureScope
import com.datadog.android.api.feature.FeatureSdkCore
import com.datadog.android.log.internal.LogsFeature
import com.datadog.android.log.internal.net.LogsRequestFactory
Expand All @@ -25,6 +27,8 @@ import org.mockito.junit.jupiter.MockitoExtension
import org.mockito.junit.jupiter.MockitoSettings
import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.eq
import org.mockito.kotlin.isNull
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
Expand Down Expand Up @@ -90,4 +94,88 @@ internal class LogsTest {
// Then
assertThat(result).isFalse
}

@Test
fun `M log user error W addAttribute { logs not enabled }`(
@StringForgery key: String,
@StringForgery value: String
) {
// Given
whenever(mockSdkCore.getFeature(Feature.LOGS_FEATURE_NAME)) doReturn null

// When
Logs.addAttribute(key, value, mockSdkCore)

// Then
argumentCaptor<() -> String> {
verify(mockSdkCore.internalLogger).log(
eq(InternalLogger.Level.ERROR),
eq(InternalLogger.Target.USER),
capture(),
isNull(),
eq(false),
eq(null)
)
assertThat(lastValue()).isEqualTo(Logs.LOGS_NOT_ENABLED_MESSAGE)
}
}

@Test
fun `M log user error W removeAttribute { logs not enabled }`(
@StringForgery key: String
) {
// Given
whenever(mockSdkCore.getFeature(Feature.LOGS_FEATURE_NAME)) doReturn null

// When
Logs.removeAttribute(key, mockSdkCore)

// Then
argumentCaptor<() -> String> {
verify(mockSdkCore.internalLogger).log(
eq(InternalLogger.Level.ERROR),
eq(InternalLogger.Target.USER),
capture(),
isNull(),
eq(false),
eq(null)
)
assertThat(lastValue()).isEqualTo(Logs.LOGS_NOT_ENABLED_MESSAGE)
}
}

@Test
fun `M forward attributes to Feature W addAttribute`(
@StringForgery key: String,
@StringForgery value: String
) {
// Given
val mockFeatureScope: FeatureScope = mock()
val mockLogsFeature: LogsFeature = mock()
whenever(mockSdkCore.getFeature(Feature.LOGS_FEATURE_NAME)) doReturn mockFeatureScope
whenever(mockFeatureScope.unwrap<LogsFeature>()) doReturn mockLogsFeature

// When
Logs.addAttribute(key, value, mockSdkCore)

// Then
verify(mockLogsFeature).addAttribute(key, value)
}

@Test
fun `M forward remove attributes to Feature W removeAttribute`(
@StringForgery key: String
) {
// Given
val mockFeatureScope: FeatureScope = mock()
val mockLogsFeature: LogsFeature = mock()
whenever(mockSdkCore.getFeature(Feature.LOGS_FEATURE_NAME)) doReturn mockFeatureScope
whenever(mockFeatureScope.unwrap<LogsFeature>()) doReturn mockLogsFeature

// When
Logs.removeAttribute(key, mockSdkCore)

// Then
verify(mockLogsFeature).removeAttribute(key)
}
}