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 telemetry-prom-merge-port flag for merged metrics #36

Merged
merged 3 commits into from
Oct 21, 2022

Conversation

curtbushko
Copy link
Contributor

@curtbushko curtbushko commented Oct 19, 2022

This PR adds a new flag to consul-dataplane named telemetry-prom-merge-port with a default value of 20100. This is in order to support merged metrics with consul-k8s. The PR is paired with the consul-k8s PR Merged metrics with consul-dataplane and this PR should be merged before consul-k8s one.

I have mainly tested the merged metrics through the consul-k8s acceptance tests and they are passing. The acceptance tests test:

  • component metrics - that you can get metrics out of the server and agent. Agent is still there as there is a chance that customers will still run agents.
  • gateways - ingress, terminating and mesh are serving metrics.
  • app - that an app can run and it's metrics are served.

@curtbushko curtbushko changed the title Add telemetry flags for merged metrics Add telemetry-prom-merge-port flag for merged metrics Oct 19, 2022
@curtbushko curtbushko added the pr/no-changelog This PR does not introduce a user-facing change that should be reflected in the changelog label Oct 19, 2022
@curtbushko curtbushko force-pushed the curtbushko/agentless-metrics-merging branch from 8f3883f to c6fc569 Compare October 19, 2022 19:10
@curtbushko curtbushko marked this pull request as ready for review October 19, 2022 19:14
@curtbushko curtbushko requested a review from a team as a code owner October 19, 2022 19:14
@curtbushko curtbushko self-assigned this Oct 19, 2022
internal/bootstrap/bootstrap_tpl.go Outdated Show resolved Hide resolved
@@ -54,6 +54,7 @@ func TestMetricsServerClosed(t *testing.T) {
}

func TestMetricsServerEnabled(t *testing.T) {
mergedMetricsBackendBindAddr := mergedMetricsBackendBindHost + defaultMergedMetricsBackendBindPort
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add tests that the metrics backend port can be overridden?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@curtbushko curtbushko requested review from ishustava, a team, samsalisbury and mdeggies and removed request for a team October 20, 2022 17:58
@curtbushko curtbushko force-pushed the curtbushko/agentless-metrics-merging branch from 9f26c3b to bd10094 Compare October 20, 2022 22:29
@curtbushko curtbushko force-pushed the curtbushko/agentless-metrics-merging branch from bd10094 to c720af1 Compare October 21, 2022 02:37
@curtbushko curtbushko merged commit 7756e5c into main Oct 21, 2022
@curtbushko curtbushko deleted the curtbushko/agentless-metrics-merging branch October 21, 2022 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-changelog This PR does not introduce a user-facing change that should be reflected in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants