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

Update the OpenTelemetry SDK version to 1.38.0 #11335

Merged
merged 5 commits into from
May 15, 2024

Conversation

opentelemetrybot
Copy link
Collaborator

Update the OpenTelemetry SDK version to 1.38.0.

@opentelemetrybot opentelemetrybot requested a review from a team as a code owner 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
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
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.

Copy link
Member

@trask trask left a comment

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
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
53 checks passed
@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
laurit added a commit to laurit/opentelemetry-java-instrumentation that referenced this pull request May 17, 2024
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.

None yet

4 participants