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

Add a new spec section to cover integration with opentelemetry metrics #623

Merged
merged 2 commits into from
Apr 25, 2024

Conversation

benjamin-confino
Copy link
Contributor

We still need a section covering what happens if both mpMetrics and mpTelemetry are live.

@benjamin-confino benjamin-confino marked this pull request as draft March 25, 2024 10:49
@eclipse-microprofile-bot
Copy link
Contributor

Can one of the admins verify this patch?


[source, java]
----
package com.exmaple;

Choose a reason for hiding this comment

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

exmaple -> example

@@ -0,0 +1,298 @@
//
// Copyright (c) 2018-2020 Contributors to the Eclipse Foundation

Choose a reason for hiding this comment

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

I think the date needs updating here


=== Scope

Metrics added by this specification will appear in the `base` MicroProfile Telemetry scope.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this [still] a thing with MP Telemetry Metrics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point

@Emily-Jiang
Copy link
Member

@benjamin-confino since this new doc is so similar to the chapter of integrating with MP Metrics, can you rework that chapter and change the heading to Metrics and then call out the difference between the integration with MP Metrics and the one with MP Telemetry?

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Apr 3, 2024

also you need to update the main doc to add a section saying "Relationship to MicroProfile Telemetry"

@benjamin-confino benjamin-confino force-pushed the spec-for-mptel-integration branch 3 times, most recently from e28d6fb to 16c006d Compare April 10, 2024 15:06
tck/pom.xml Outdated Show resolved Hide resolved
@benjamin-confino benjamin-confino force-pushed the spec-for-mptel-integration branch 8 times, most recently from b5cd00b to 5ab92d7 Compare April 17, 2024 14:59
tck/pom.xml Outdated Show resolved Hide resolved
@Azquelt
Copy link
Member

Azquelt commented Apr 18, 2024

@eclipse-microprofile-bot test this please

@benjamin-confino benjamin-confino marked this pull request as ready for review April 18, 2024 10:22
@benjamin-confino benjamin-confino force-pushed the spec-for-mptel-integration branch from 5f97f23 to a8edd0c Compare April 18, 2024 12:43
@benjamin-confino benjamin-confino force-pushed the spec-for-mptel-integration branch 3 times, most recently from 4112ca4 to f5a3f5d Compare April 18, 2024 13:02
api/pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@benjamin-confino benjamin-confino force-pushed the spec-for-mptel-integration branch 3 times, most recently from 463b11a to 46cd8fc Compare April 22, 2024 14:39
@benjamin-confino benjamin-confino force-pushed the spec-for-mptel-integration branch from 46cd8fc to 533df9b Compare April 22, 2024 14:51
@Emily-Jiang
Copy link
Member

@benjamin-confino please update the copyright dates accordingly. Some new files has the wrong copyright year.

@Azquelt
Copy link
Member

Azquelt commented Apr 23, 2024

@Emily-Jiang Which files are wrong? In some cases Ben has copied an existing MP Metrics test and made modifications for Telemetry so the old date was retained although the file appears as new in the diff. I did check for wrong dates and didn't spot any.

Somewhat related: I did spot some files where the second date in the range should potentially be updated, but the handbook now says we only need the original date anyway. I was planning to remove the second date from all files to match the handbook in another PR.

@Emily-Jiang
Copy link
Member

@Emily-Jiang Which files are wrong? In some cases Ben has copied an existing MP Metrics test and made modifications for Telemetry so the old date was retained although the file appears as new in the diff. I did check for wrong dates and didn't spot any.

Somewhat related: I did spot some files where the second date in the range should potentially be updated, but the handbook now says we only need the original date anyway. I was planning to remove the second date from all files to match the handbook in another PR.

You were right. I was commented on the current year. Based on the new version of the handbook, the year of the changes are no longer to be updated. Well spotted. I have approved this PR and you can merge it and do the copyright end year via this issue.

Copy link
Member

@Emily-Jiang Emily-Jiang left a comment

Choose a reason for hiding this comment

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

LGTM

@Azquelt Azquelt merged commit 2db3341 into eclipse:main Apr 25, 2024
2 checks passed
@Azquelt Azquelt linked an issue Aug 7, 2024 that may be closed by this pull request
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Work with MicroProfile Telemetry Metrics
6 participants