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

Revisit metric exports #5675

Open
jvalkeal opened this issue Feb 13, 2024 · 2 comments
Open

Revisit metric exports #5675

jvalkeal opened this issue Feb 13, 2024 · 2 comments
Assignees
@jvalkeal
Milestone
3.0.x

Comments

@jvalkeal
Copy link
Collaborator

jvalkeal commented Feb 13, 2024 •
edited

This is a short recap when going through various issues:

  • spring-cloud-dataflow-parent/pom.xml manages wavefront-spring-boot-bom.version as 2.3.4. Bit nasty as boot doesn't manage wavefront but depends something which then pulls specific wavefront sdk version. Should use 3.2.0 and keep that aligned.
  • Our server configs currently use influx, prometheus and wavefront and export is disabled on default. Boot changed property format in spring-projects/spring-boot@be3523b, we need to change property names. In short management.metrics.export.prometheus.enabled vs. management.prometheus.metrics.export.enabled.
  • However there's micrometer-metrics/prometheus-rsocket-proxy which still uses management.metrics.export.prometheus.rsocket.enabled, they never migrated to new property structure.
  • Thought there's no "release" in prometheus-rsocket-proxy claiming boot 3.x support so wondering if we should just remove this dependency.
@jvalkeal jvalkeal added this to the 3.0.x milestone Feb 13, 2024
@jvalkeal jvalkeal self-assigned this Feb 13, 2024
@onobc
Copy link
Contributor

onobc commented Feb 13, 2024 •
edited

Thanks for the details @jvalkeal,

Our server configs currently use influx, prometheus and wavefront and export is disabled on default. Boot changed property format in spring-projects/spring-boot@be3523b, we need to change property names. In short management.metrics.export.prometheus.enabled vs. management.prometheus.metrics.export.enabled.

We already account for this in the MetricsReplicationEnvironmentPostProcessor. Take a look. This technique allowed us to support both Boot2 and Boot3 apps. However, we are not supporting Boot2 apps going forward in SCDF3.x so this could be refactored/removed/adjusted if needed but it may be easier to leave it as is and move onto the next item in the above list.

Please also see this section in the ref guide about this: https://docs.spring.io/spring-cloud-dataflow/docs/current/reference/htmlsingle/#_metrics_configuration_properties

However there's micrometer-metrics/prometheus-rsocket-proxy which still uses management.metrics.export.prometheus.rsocket.enabled, they never migrated to new property structure.

Haha. Yep, I have been down this same road you are on w/ the Boot2/3 stuff.

Thought there's no "release" in prometheus-rsocket-proxy claiming boot 3.x support so wondering if we should just remove this dependency.

If we remove it, what "hole" will that leave functionally?

@jvalkeal
Copy link
Collaborator Author

Folks from micrometer were open to do some further polish in prometheus-rsocket-proxy and release it having boot3 support. So we can temporarily keep it in current state as they probably overhaul their properties for enable state.

jvalkeal added a commit to jvalkeal/spring-cloud-dataflow that referenced this issue Feb 20, 2024
@jvalkeal
Overhaul metric deps and options
ec31ed5 
Align wavefront version so that we don't have misaligned versions
coming out from other parts of a metric system. Short story
is that boot doesn't manage wavefront but there is an explicit
dependency to wavefront sdk libs in metric system.

Rename metric options within management for influx, prometheus
and wavefront to align changes in boot itself.

For rsocket proxy keep old management.metrics.export.prometheus.rsocket.enabled
as that is going to get moved under micrometer spesific namespace when
they release boot3 support.

This commit was supposed to get skipper server to start but there
is a new issue about missing bean
org.springframework.statemachine.data.jpa.JpaStateMachineRepository
which will need to get fixed in a separate commit.

Fixes spring-cloud#5675
@jvalkeal jvalkeal mentioned this issue Feb 20, 2024
Closed
cppwfs pushed a commit to cppwfs/spring-cloud-dataflow that referenced this issue Feb 20, 2024
@jvalkeal @cppwfs
Overhaul metric deps and options
3cdc12f 
Align wavefront version so that we don't have misaligned versions
coming out from other parts of a metric system. Short story
is that boot doesn't manage wavefront but there is an explicit
dependency to wavefront sdk libs in metric system.

Rename metric options within management for influx, prometheus
and wavefront to align changes in boot itself.

For rsocket proxy keep old management.metrics.export.prometheus.rsocket.enabled
as that is going to get moved under micrometer spesific namespace when
they release boot3 support.

This commit was supposed to get skipper server to start but there
is a new issue about missing bean
org.springframework.statemachine.data.jpa.JpaStateMachineRepository
which will need to get fixed in a separate commit.

Fixes spring-cloud#5675
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jvalkeal jvalkeal

Labels
None yet
Projects
None yet
Milestone
3.0.x
Development

No branches or pull requests
2 participants
@jvalkeal @onobc