Skip to content

Implementing ExtendedLogRecordData #1918

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

Merged
merged 2 commits into from
Jun 4, 2025
+15 −2
Merged

Implementing ExtendedLogRecordData #1918

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7effb0d
Implementing ExtendedLogRecordData
LikeTheSalad May 28, 2025
9cf7aac
Updating suppression warning annotation and adding a description message
LikeTheSalad May 29, 2025
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
1 change: 1 addition & 0 deletions disk-buffering/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ val protos by configurations.creating

dependencies {
api("io.opentelemetry:opentelemetry-sdk")
implementation("io.opentelemetry:opentelemetry-api-incubator")
compileOnly("com.google.auto.value:auto-value-annotations")
annotationProcessor("com.google.auto.value:auto-value")
signature("com.toasttab.android:gummy-bears-api-21:0.6.1:coreLib@signature")
Expand Down
14 changes: 12 additions & 2 deletions .../contrib/disk/buffering/internal/serialization/mapping/logs/models/LogRecordDataImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,17 @@
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.Value;
import io.opentelemetry.api.incubator.common.ExtendedAttributes;
import io.opentelemetry.api.logs.Severity;
import io.opentelemetry.api.trace.SpanContext;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import io.opentelemetry.sdk.logs.data.LogRecordData;
import io.opentelemetry.sdk.logs.data.internal.ExtendedLogRecordData;
import io.opentelemetry.sdk.resources.Resource;
import javax.annotation.Nullable;

@SuppressWarnings({"deprecation", "SuppressWarningsWithoutExplanation"})
Copy link
Contributor

@laurit laurit May 29, 2025

Choose a reason for hiding this comment

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

Instead of SuppressWarningsWithoutExplanation we usually add the explanation. If possible move the deprecation closer to where the deprecated method is used.
Looking at the sdk code there are 2 implementations for these classes. The extended one when the incubator is available and the non-extended one that is used otherwise. Did you consider using the same pattern?

Copy link
Contributor Author

@LikeTheSalad LikeTheSalad May 29, 2025

Choose a reason for hiding this comment

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

Instead of SuppressWarningsWithoutExplanation we usually add the explanation. If possible move the deprecation closer to where the deprecated method is used.

Got it. I've just updated it.

Looking at the sdk code there are 2 implementations for these classes. The extended one when the incubator is available and the non-extended one that is used otherwise. Did you consider using the same pattern?

I think that's the approach we should take when/if we add early support for extended attributes in this module. I'm not sure for how long this feature is meant to stay in an incubating state, which is something I wanted to check in the issue thread but so far I haven't gotten feedback on that, so that if it's not too long, maybe we won't need to create 2 impls if we can just wait for the stable interface to get extended. For now, my goal with these changes is to at least keep the "old/stable impl" working in the meantime, since all logs are broken atm.

@AutoValue
public abstract class LogRecordDataImpl implements LogRecordData {
public abstract class LogRecordDataImpl implements ExtendedLogRecordData {

public static Builder builder() {
return new AutoValue_LogRecordDataImpl.Builder();
Expand All @@ -31,6 +33,14 @@ public io.opentelemetry.sdk.logs.data.Body getBody() {
: io.opentelemetry.sdk.logs.data.Body.string(valueBody.asString());
}

@Override
public ExtendedAttributes getExtendedAttributes() {
return ExtendedAttributes.builder().putAll(getAttributes()).build();
}

@Override
public abstract Attributes getAttributes();

@Override
@Nullable
public abstract Value<?> getBodyValue();
Expand Down
Loading