Skip to content

Extend Logs API to allow passing in attributes #4402

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

Merged
merged 5 commits into from
May 22, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -4712,6 +4712,8 @@ public abstract interface class io/sentry/logger/ILoggerApi {
public abstract fun info (Ljava/lang/String;[Ljava/lang/Object;)V
public abstract fun log (Lio/sentry/SentryLogLevel;Lio/sentry/SentryDate;Ljava/lang/String;[Ljava/lang/Object;)V
public abstract fun log (Lio/sentry/SentryLogLevel;Ljava/lang/String;[Ljava/lang/Object;)V
public abstract fun log (Ljava/util/Map;Lio/sentry/SentryLogLevel;Lio/sentry/SentryDate;Ljava/lang/String;[Ljava/lang/Object;)V
public abstract fun log (Ljava/util/Map;Lio/sentry/SentryLogLevel;Ljava/lang/String;[Ljava/lang/Object;)V
public abstract fun trace (Ljava/lang/String;[Ljava/lang/Object;)V
public abstract fun warn (Ljava/lang/String;[Ljava/lang/Object;)V
}
Expand All @@ -4729,6 +4731,8 @@ public final class io/sentry/logger/LoggerApi : io/sentry/logger/ILoggerApi {
public fun info (Ljava/lang/String;[Ljava/lang/Object;)V
public fun log (Lio/sentry/SentryLogLevel;Lio/sentry/SentryDate;Ljava/lang/String;[Ljava/lang/Object;)V
public fun log (Lio/sentry/SentryLogLevel;Ljava/lang/String;[Ljava/lang/Object;)V
public fun log (Ljava/util/Map;Lio/sentry/SentryLogLevel;Lio/sentry/SentryDate;Ljava/lang/String;[Ljava/lang/Object;)V
public fun log (Ljava/util/Map;Lio/sentry/SentryLogLevel;Ljava/lang/String;[Ljava/lang/Object;)V
public fun trace (Ljava/lang/String;[Ljava/lang/Object;)V
public fun warn (Ljava/lang/String;[Ljava/lang/Object;)V
}
Expand All @@ -4749,6 +4753,8 @@ public final class io/sentry/logger/NoOpLoggerApi : io/sentry/logger/ILoggerApi
public fun info (Ljava/lang/String;[Ljava/lang/Object;)V
public fun log (Lio/sentry/SentryLogLevel;Lio/sentry/SentryDate;Ljava/lang/String;[Ljava/lang/Object;)V
public fun log (Lio/sentry/SentryLogLevel;Ljava/lang/String;[Ljava/lang/Object;)V
public fun log (Ljava/util/Map;Lio/sentry/SentryLogLevel;Lio/sentry/SentryDate;Ljava/lang/String;[Ljava/lang/Object;)V
public fun log (Ljava/util/Map;Lio/sentry/SentryLogLevel;Ljava/lang/String;[Ljava/lang/Object;)V
public fun trace (Ljava/lang/String;[Ljava/lang/Object;)V
public fun warn (Ljava/lang/String;[Ljava/lang/Object;)V
}
Expand Down
14 changes: 14 additions & 0 deletions sentry/src/main/java/io/sentry/logger/ILoggerApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io.sentry.SentryDate;
import io.sentry.SentryLogLevel;
import java.util.Map;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -28,4 +29,17 @@ void log(
@Nullable SentryDate timestamp,
@Nullable String message,
@Nullable Object... args);

void log(
@Nullable Map<String, Object> attributes,
@NotNull SentryLogLevel level,
@Nullable String message,
@Nullable Object... args);

void log(
@Nullable Map<String, Object> attributes,
@NotNull SentryLogLevel level,
@Nullable SentryDate timestamp,
@Nullable String message,
@Nullable Object... args);
}
41 changes: 37 additions & 4 deletions sentry/src/main/java/io/sentry/logger/LoggerApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.sentry.util.Platform;
import io.sentry.util.TracingUtils;
import java.util.HashMap;
import java.util.Map;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -65,7 +66,7 @@ public void log(
final @NotNull SentryLogLevel level,
final @Nullable String message,
final @Nullable Object... args) {
log(level, null, message, args);
captureLog(null, level, null, message, args);
}

@Override
Expand All @@ -74,11 +75,31 @@ public void log(
final @Nullable SentryDate timestamp,
final @Nullable String message,
final @Nullable Object... args) {
captureLog(level, timestamp, message, args);
captureLog(null, level, timestamp, message, args);
}

@Override
public void log(
final @Nullable Map<String, Object> attributes,
final @NotNull SentryLogLevel level,
final @Nullable SentryDate timestamp,
final @Nullable String message,
final @Nullable Object... args) {
captureLog(attributes, level, timestamp, message, args);
}

@Override
public void log(
final @Nullable Map<String, Object> attributes,
final @NotNull SentryLogLevel level,
final @Nullable String message,
final @Nullable Object... args) {
captureLog(attributes, level, null, message, args);
}

@SuppressWarnings("AnnotateFormatMethod")
private void captureLog(
final @Nullable Map<String, Object> attributes,
Copy link
Member

Choose a reason for hiding this comment

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

I don't like it too much that it's the first param but I guess it must be like this because of the varargs right?

Copy link
Member

Choose a reason for hiding this comment

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

I would like it more for this to be the last param but I don't think that's possible. Either way it's fine, I would expect most users to not go through this API

final @NotNull SentryLogLevel level,
final @Nullable SentryDate timestamp,
final @Nullable String message,
Expand Down Expand Up @@ -119,7 +140,7 @@ private void captureLog(
span == null ? propagationContext.getSpanId() : span.getSpanContext().getSpanId();
final SentryLogEvent logEvent =
new SentryLogEvent(traceId, timestampToUse, messageToUse, level);
logEvent.setAttributes(createAttributes(message, spanId, args));
logEvent.setAttributes(createAttributes(attributes, message, spanId, args));
logEvent.setSeverityNumber(level.getSeverityNumber());

scopes.getClient().captureLog(logEvent, combinedScope);
Expand All @@ -146,8 +167,20 @@ private void captureLog(
}

private @NotNull HashMap<String, SentryLogEventAttributeValue> createAttributes(
final @NotNull String message, final @NotNull SpanId spanId, final @Nullable Object... args) {
final @Nullable Map<String, Object> incomingAttributes,
final @NotNull String message,
final @NotNull SpanId spanId,
final @Nullable Object... args) {
final @NotNull HashMap<String, SentryLogEventAttributeValue> attributes = new HashMap<>();

if (incomingAttributes != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we create a copy of the attributes (here or in the capture method) to avoid having a map that could be changed while we iterate on it?

for (Map.Entry<String, Object> attributeEntry : incomingAttributes.entrySet()) {
final @Nullable Object value = attributeEntry.getValue();
final @NotNull String type = getType(value);
attributes.put(attributeEntry.getKey(), new SentryLogEventAttributeValue(type, value));
}
}

if (args != null) {
int i = 0;
for (Object arg : args) {
Expand Down
20 changes: 20 additions & 0 deletions sentry/src/main/java/io/sentry/logger/NoOpLoggerApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io.sentry.SentryDate;
import io.sentry.SentryLogLevel;
import java.util.Map;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -61,4 +62,23 @@ public void log(
@Nullable Object... args) {
// do nothing
}

@Override
public void log(
@Nullable Map<String, Object> attributes,
@NotNull SentryLogLevel level,
@Nullable String message,
@Nullable Object... args) {
// do nothing
}

@Override
public void log(
@Nullable Map<String, Object> attributes,
@NotNull SentryLogLevel level,
@Nullable SentryDate timestamp,
@Nullable String message,
@Nullable Object... args) {
// do nothing
}
}
107 changes: 107 additions & 0 deletions sentry/src/test/java/io/sentry/ScopesTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2628,6 +2628,113 @@ class ScopesTest {
)
}

@Test
fun `creating log with timestamp works`() {
val (sut, mockClient) = getEnabledScopes {
it.logs.isEnabled = true
}

sut.logger().log(SentryLogLevel.WARN, SentryLongDate(123), "log message")

verify(mockClient).captureLog(
check {
assertEquals("log message", it.body)
assertEquals(0.000000123, it.timestamp)
assertEquals(SentryLogLevel.WARN, it.level)
assertEquals(13, it.severityNumber)
},
anyOrNull()
)
}

@Test
fun `creating log with attributes works`() {
val (sut, mockClient) = getEnabledScopes {
it.logs.isEnabled = true
}

sut.logger().log(mapOf("attrname1" to "attrval1"), SentryLogLevel.WARN, "log message")

verify(mockClient).captureLog(
check {
assertEquals("log message", it.body)
assertEquals(SentryLogLevel.WARN, it.level)
assertEquals(13, it.severityNumber)

val attr1 = it.attributes?.get("attrname1")!!
assertEquals("attrval1", attr1.value)
assertEquals("string", attr1.type)
},
anyOrNull()
)
}

@Test
fun `creating log with attributes and timestamp works`() {
val (sut, mockClient) = getEnabledScopes {
it.logs.isEnabled = true
}

sut.logger().log(mapOf("attrname1" to "attrval1"), SentryLogLevel.WARN, SentryLongDate(123), "log message")

verify(mockClient).captureLog(
check {
assertEquals("log message", it.body)
assertEquals(0.000000123, it.timestamp)
assertEquals(SentryLogLevel.WARN, it.level)
assertEquals(13, it.severityNumber)

val attr1 = it.attributes?.get("attrname1")!!
assertEquals("attrval1", attr1.value)
assertEquals("string", attr1.type)
},
anyOrNull()
)
}

@Test
fun `creating log with attributes and timestamp and format string works`() {
val (sut, mockClient) = getEnabledScopes {
it.logs.isEnabled = true
}

sut.logger().log(mapOf("attrname1" to "attrval1"), SentryLogLevel.WARN, SentryLongDate(123), "log %s %d %b %.0f", "message", 1, true, 3.2)

verify(mockClient).captureLog(
check {
assertEquals("log message 1 true 3", it.body)
assertEquals(0.000000123, it.timestamp)
assertEquals(SentryLogLevel.WARN, it.level)
assertEquals(13, it.severityNumber)

val attr1 = it.attributes?.get("attrname1")!!
assertEquals("attrval1", attr1.value)
assertEquals("string", attr1.type)

val template = it.attributes?.get("sentry.message.template")!!
assertEquals("log %s %d %b %.0f", template.value)
assertEquals("string", template.type)

val param0 = it.attributes?.get("sentry.message.parameter.0")!!
assertEquals("message", param0.value)
assertEquals("string", param0.type)

val param1 = it.attributes?.get("sentry.message.parameter.1")!!
assertEquals(1, param1.value)
assertEquals("integer", param1.type)

val param2 = it.attributes?.get("sentry.message.parameter.2")!!
assertEquals(true, param2.value)
assertEquals("boolean", param2.type)

val param3 = it.attributes?.get("sentry.message.parameter.3")!!
assertEquals(3.2, param3.value)
assertEquals("double", param3.type)
},
anyOrNull()
)
}

@Test
fun `creating log with without args does not add template attribute`() {
val (sut, mockClient) = getEnabledScopes {
Expand Down
Loading