Skip to content

Update the OpenTelemetry SDK version to 1.38.0#11335

Merged
trask merged 5 commits into
mainfrom
opentelemetrybot/update-opentelemetry-sdk-to-1.38.0
May 15, 2024
Merged

Update the OpenTelemetry SDK version to 1.38.0#11335
trask merged 5 commits into
mainfrom
opentelemetrybot/update-opentelemetry-sdk-to-1.38.0

Conversation

@opentelemetrybot

Copy link
Copy Markdown
Contributor

Update the OpenTelemetry SDK version to 1.38.0.

@opentelemetrybot opentelemetrybot requested a review from a team May 10, 2024 15:53
@github-actions github-actions Bot added the test native This label can be applied to PRs to trigger them to run native tests label May 10, 2024
@github-actions github-actions Bot requested a review from theletterf May 10, 2024 15:53
.hasAttributesSatisfying(
equalTo(stringKey("test"), "test"))))));

// sleep exporter interval

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please explain the reason of this sleep and the following one?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This appears to be a standard pattern that is used in these tests. It was already present in the original version of this test (this test is a slightly modified copy of the same test in v1_37 which is a copy from another version). My understanding is that the first sleep is there to ensure that metric data arrives before clearData is called, but as there is already a call to waitAndAssertMetrics that also waits for metrics to arrive (actually it waits for assertion to pass, but here assertion requires metrics) I don't think it is really necessary. The second sleep after clearData should ensure that if there is new metrics are produced they have time to arrive. I think this is necessary because the following waitAndAssertMetrics asserts that there are no metrics and this could pass if we don't wait until metrics have had time to arrive (assuming that there is a bug and there unexpectedly are metrics).
Changing these is out of the scope for this PR.

@trask trask left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks ❤️

import org.junit.jupiter.api.TestInfo;
import org.junit.jupiter.api.extension.RegisterExtension;

class MeterTest {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what do you think of extending an AbstractMeterTest so we can get coverage of all the normal metrics behavior as well, or do you think it's not really needed?

@trask trask merged commit 772cbd5 into main May 15, 2024
@trask trask deleted the opentelemetrybot/update-opentelemetry-sdk-to-1.38.0 branch May 15, 2024 18:07
laurit added a commit to laurit/opentelemetry-java-instrumentation that referenced this pull request May 17, 2024
Co-authored-by: Lauri Tulmin <ltulmin@splunk.com>
laurit added a commit to laurit/opentelemetry-java-instrumentation that referenced this pull request May 17, 2024
Co-authored-by: Lauri Tulmin <ltulmin@splunk.com>
laurit added a commit to laurit/opentelemetry-java-instrumentation that referenced this pull request May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test native This label can be applied to PRs to trigger them to run native tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants