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.

Sign up for GitHub

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

Jump to bottom

Preserve attribute type for logback key value pairs #10781

Merged
merged 3 commits into from
Mar 12, 2024
Merged

Preserve attribute type for logback key value pairs #10781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d82460d
Preserve attribute type for logback key value pairs
laurit Mar 8, 2024
6125d33
add comment
laurit Mar 11, 2024
2841e1b
Merge branch 'main' into logback-preserve-type
laurit Mar 11, 2024
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
17 changes: 15 additions & 2 deletions ...a/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,21 @@ private static void captureKeyValuePairAttributes(
List<KeyValuePair> keyValuePairs = loggingEvent.getKeyValuePairs();
if (keyValuePairs != null) {
for (KeyValuePair keyValuePair : keyValuePairs) {
Object value = keyValuePair.value;
if (keyValuePair.value != null) {
attributes.put(getAttributeKey(keyValuePair.key), keyValuePair.value.toString());
// preserve type for boolean and numeric values, everything else is converted to String
if (value instanceof Boolean) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Might it be more friendly for code reading by adding some comments explaining what types are processed now?

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'm not sure what comment you are asking for.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what comment you are asking for.

I mean we can add some comment to describe what types are supported and what types are not supported to preserve attribute type now.

attributes.put(keyValuePair.key, (Boolean) keyValuePair.value);
} else if (value instanceof Byte
|| value instanceof Integer
|| value instanceof Long
|| value instanceof Short) {
attributes.put(keyValuePair.key, ((Number) keyValuePair.value).longValue());
} else if (value instanceof Double || value instanceof Float) {
attributes.put(keyValuePair.key, ((Number) keyValuePair.value).doubleValue());
} else {
attributes.put(getAttributeKey(keyValuePair.key), keyValuePair.value.toString());
}
}
}
}
Expand All @@ -236,7 +249,7 @@ private static void captureLoggerContext(
}

public static AttributeKey<String> getAttributeKey(String key) {
return attributeKeys.computeIfAbsent(key, k -> AttributeKey.stringKey(k));
return attributeKeys.computeIfAbsent(key, AttributeKey::stringKey);
}

private static boolean supportsKeyValuePairs() {
Expand Down
30 changes: 25 additions & 5 deletions ...slf4j2ApiTest/java/io/opentelemetry/instrumentation/logback/appender/v1_0/Slf4j2Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.instrumentation.logback.appender.v1_0;

import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.sdk.OpenTelemetrySdk;
Expand Down Expand Up @@ -55,18 +56,37 @@ void setup() {

@Test
void keyValue() {
logger.atInfo().setMessage("log message 1").addKeyValue("key", "value").log();
logger
.atInfo()
.setMessage("log message 1")
.addKeyValue("string key", "string value")
.addKeyValue("boolean key", true)
.addKeyValue("byte key", (byte) 1)
.addKeyValue("short key", (short) 2)
.addKeyValue("int key", 3)
.addKeyValue("long key", 4L)
.addKeyValue("float key", 5.0f)
.addKeyValue("double key", 6.0)
.log();

List<LogRecordData> logDataList = logRecordExporter.getFinishedLogRecordItems();
assertThat(logDataList).hasSize(1);
LogRecordData logData = logDataList.get(0);
assertThat(logData.getResource()).isEqualTo(resource);
assertThat(logData.getInstrumentationScopeInfo()).isEqualTo(instrumentationScopeInfo);
assertThat(logData.getBody().asString()).isEqualTo("log message 1");
assertThat(logData.getAttributes().size()).isEqualTo(5); // 4 code attributes + 1 key value pair
assertThat(logData.getAttributes())
.hasEntrySatisfying(
AttributeKey.stringKey("key"), value -> assertThat(value).isEqualTo("value"));
assertThat(logData.getAttributes().size())
.isEqualTo(12); // 4 code attributes + 8 key value pairs
assertThat(logData)
.hasAttributesSatisfying(
equalTo(AttributeKey.stringKey("string key"), "string value"),
equalTo(AttributeKey.booleanKey("boolean key"), true),
equalTo(AttributeKey.longKey("byte key"), 1),
equalTo(AttributeKey.longKey("short key"), 2),
equalTo(AttributeKey.longKey("int key"), 3),
equalTo(AttributeKey.longKey("long key"), 4),
equalTo(AttributeKey.doubleKey("float key"), 5.0),
equalTo(AttributeKey.doubleKey("double key"), 6.0));
}

@Test
Expand Down
Loading