Skip to content
This repository was archived by the owner on Dec 12, 2025. It is now read-only.

Conversation

@rodrigovalin
Copy link
Contributor

@rodrigovalin rodrigovalin commented Feb 9, 2022

Summary

Using latest agent, we can pass a spec.metrics.prometheus configuration that will result in an automation_config change that enables a /metrics endpoint that can be used with Prometheus.

Dependencies

We need to publish a new agent version before these changes are even recognized by the agents.

@rodrigovalin rodrigovalin force-pushed the CLOUDP-112098_configure_prometheus branch 2 times, most recently from 40ecfc8 to ca62366 Compare February 9, 2022 17:06
@rodrigovalin rodrigovalin force-pushed the CLOUDP-112098_configure_prometheus branch from ca62366 to 3c7ff35 Compare February 16, 2022 14:51
Copy link
Contributor

@priyolahiri priyolahiri left a comment

Choose a reason for hiding this comment

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

LGTM

AuthenticationRestrictions []AuthenticationRestriction `json:"authenticationRestrictions,omitempty"`
}

type Metrics struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: Any reason to wrap this in Metrics struct instead of using Prometheus struct directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason in particular, maybe preparing for other options in the future, to go besides Prometheus; but I think that posibility is unlikely.

I don't have a strong opinion. Maybe it is unecessary at this point. I will remove

Username: username,
Scheme: "http",
Mode: "opsManager",
ListenAddress: "0.0.0.0:9216",
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 hardcode the port here? can it be changed on some usecase?

Copy link
Contributor Author

@rodrigovalin rodrigovalin Feb 17, 2022

Choose a reason for hiding this comment

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

It can be changed from CRD via prometheus.port, this is just the default.

Copy link
Contributor

@irajdeep irajdeep left a comment

Choose a reason for hiding this comment

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

LGTM -- added some minor comments.

@rodrigovalin rodrigovalin force-pushed the CLOUDP-112098_configure_prometheus branch from 068bc5e to 295f9b9 Compare February 17, 2022 12:34
@rodrigovalin rodrigovalin merged commit c4f22d2 into master Feb 17, 2022
@rodrigovalin rodrigovalin deleted the CLOUDP-112098_configure_prometheus branch February 17, 2022 16:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants