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.

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

Append /v1/metrics to endpoint path if the protocol is HTTP #284

Merged
merged 5 commits into from
Sep 29, 2022
Merged

Append /v1/metrics to endpoint path if the protocol is HTTP #284

merged 5 commits into from
Sep 29, 2022

Conversation

pkanal
Copy link
Contributor

@pkanal pkanal commented Sep 28, 2022

Which problem is this PR solving?

This PR follows up on #279 and adds the same logic to append /v1/metrics path to the endpoint if the protocol is http/protobuf or http/json

Short description of the changes

  • Adds smoke tests for metrics
  • Consolidates tests with many cases using InlineData
  • Adds logic to append /v1/metrics to the generic endpoint if a path is not already specified

@pkanal pkanal marked this pull request as ready for review September 28, 2022 19:59
@pkanal pkanal requested review from a team and robbkidd September 28, 2022 19:59
@pkanal pkanal added the type: bug Something isn't working label Sep 28, 2022
@pkanal pkanal self-assigned this Sep 28, 2022
MikeGoldsmith
MikeGoldsmith approved these changes Sep 29, 2022
View reviewed changes
Copy link
Contributor

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @pkanal 👍🏻

@JamieDanielson JamieDanielson added the version: bump minor A PR that adds behavior, but is backwards-compatible. label Sep 29, 2022
@pkanal pkanal merged commit d659e66 into main Sep 29, 2022
@pkanal pkanal deleted the purvi/metrics-path branch September 29, 2022 15:07
@pkanal pkanal mentioned this pull request Sep 29, 2022
Merged
@JamieDanielson JamieDanielson mentioned this pull request Dec 9, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TylerHelmuth TylerHelmuth TylerHelmuth left review comments

@MikeGoldsmith MikeGoldsmith MikeGoldsmith approved these changes

@robbkidd robbkidd Awaiting requested review from robbkidd

Assignees

@pkanal pkanal

Labels
type: bug Something isn't working version: bump minor A PR that adds behavior, but is backwards-compatible.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pkanal @MikeGoldsmith @TylerHelmuth @JamieDanielson