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 release notes for metrics changes #548

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

Azquelt
Copy link
Member

@Azquelt Azquelt commented May 20, 2020

Add release notes for #401 and #499

I'm not exactly sure of the difference between functional changes and spec changes are, but we seem to list changes under everything they apply to.

I've copied the way that MP Metrics denote breaking changes as it seemed fairly sensible.

@Ladicek
Copy link
Contributor

Ladicek commented May 20, 2020

When I was doing release notes for MP FT 2.1, I didn't understand the difference either. I think we should just get rid of "functional changes".

@Emily-Jiang
Copy link
Member

A new paragraph needs to be added to document the breaking changes.

@Azquelt
Copy link
Member Author

Azquelt commented Jun 2, 2020

@Emily-Jiang I've updated the release notes to include a section for backward incompatible changes with more detail of what's changed and what's needed to migrate.

spec/src/main/asciidoc/release_notes.asciidoc Outdated Show resolved Hide resolved
spec/src/main/asciidoc/release_notes.asciidoc Outdated Show resolved Hide resolved
The names of the metrics added automatically by MicroProfile Fault Tolerance have been updated to take advantage of support for metric tags which was added to MicroProfile Metrics in version 2.0. As a result, some information which was previously contained in the metric name is now instead included in tags.

In addition, metrics have moved from the `application:` scope to the `base:` scope for consistency with other MicroProfile specifications.

Copy link
Member

Choose a reason for hiding this comment

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

Previously you see the metrics under /metrics and /metrics/application. With this change, the metrics will be displayed under /metrics endpoint only.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I want to add this information. In other parts of the spec, we only talk about how fault tolerance adds metrics through the Metrics API, we never discuss or test how MP Metrics exports that information (which indirectly insulates us from any changes they might make to the export format).

Additionally, if we do add it, then, as far as I'm aware, it makes the following changes:

  • metrics are no longer exported from /metrics/application
  • metrics are exported from /metrics/base
  • in the JSON format, when metrics are retrieved from /metrics they appear in the base object rather than the application object
  • in the OpenMetrics format, the names are prefixed with base_ instead of application_

Copy link
Member

Choose a reason for hiding this comment

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

The spec does not talk about the difference between the endpoints, but MP Metrics has the details. I would vote to add what you proposed above. It is crystal clear from an end user point of view.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I've added it, but I'm not sure if it means we devote too much space to talking about the scope change, when the change of metric names and the addition of tags is the bigger and more important part of the change.

I'm happy to leave it in or take it out again, let me know what you think.

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.

See my inline comments

@Azquelt Azquelt force-pushed the metrics-release-notes branch 2 times, most recently from 1a7e658 to 126ea30 Compare June 8, 2020 15:08
spec/src/main/asciidoc/release_notes.asciidoc Outdated Show resolved Hide resolved
spec/src/main/asciidoc/release_notes.asciidoc Outdated Show resolved Hide resolved
spec/src/main/asciidoc/release_notes.asciidoc Outdated Show resolved Hide resolved
Signed-off-by: Andrew Rouse <anrouse@uk.ibm.com>
@Azquelt Azquelt merged commit 677e4ee into eclipse:master Jun 9, 2020
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.

None yet

3 participants