Skip to content

Update OTLPMetric test to latest OTel SDK #1071

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
Merged
merged 1 commit into from
Jan 30, 2023
Merged

Update OTLPMetric test to latest OTel SDK #1071

merged 1 commit into from
Jan 30, 2023

Conversation

@jefferbrecht
Copy link
Member

@jefferbrecht jefferbrecht commented Jan 30, 2023

Description

Context: open-telemetry/opentelemetry-go#3575

Related issue

b/267152693

How has this been tested?

Tested the new metrics.go standalone until it worked; will let the presubmits run the actual integration test.

Checklist:

  • Unit tests
    • Unit tests do not apply.
    • Unit tests have been added/modified and passed for this PR.
  • Integration tests
    • Integration tests do not apply.
    • Integration tests have been added/modified and passed for this PR.
  • Documentation
    • This PR introduces no user visible changes.
    • This PR introduces user visible changes and the corresponding documentation change has been made.
  • Minor version bump
    • This PR introduces no new features.
    • This PR introduces new features, and there is a separate PR to bump the minor version since the last release already.
    • This PR bumps the version.

@jefferbrecht jefferbrecht requested review from a team and franciscovalentecastro and removed request for a team January 30, 2023 14:53
franciscovalentecastro
franciscovalentecastro approved these changes Jan 30, 2023
View reviewed changes
integration_test/testdata/otlp/metrics.go

func testGaugeMetric(meter metric.Meter, name string) {
gauge, err := meter.AsyncFloat64().Gauge(name)
gauge, err := meter.Float64ObservableGauge(name)
Copy link
Contributor

@franciscovalentecastro franciscovalentecastro Jan 30, 2023

Choose a reason for hiding this comment

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

What's the difference between the previous Async / Sync to the current methods ?

Copy link
Member Author

@jefferbrecht jefferbrecht Jan 30, 2023

Choose a reason for hiding this comment

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

AFAICT they've just been renamed -- Sync is now omitted from the name and Async instruments are now Observable.

@franciscovalentecastro
Copy link
Contributor

franciscovalentecastro commented Jan 30, 2023

Optional : Change the name of the PR or commit when merged, so it mentions it's only an update in the OTLPMetric test.

@jefferbrecht jefferbrecht changed the title Update to latest OTel SDK Update OTLPMetric test to latest OTel SDK Jan 30, 2023
@jefferbrecht jefferbrecht merged commit a200ece into master Jan 30, 2023
@jefferbrecht jefferbrecht deleted the jefferbrecht-otlp-test-fix branch January 30, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@franciscovalentecastro franciscovalentecastro franciscovalentecastro approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jefferbrecht @franciscovalentecastro