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 ability to include/exclude part of a composite contributor in a health group #23027

Closed
christophejan opened this issue Aug 20, 2020 · 4 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@christophejan
Copy link
Contributor

Since #19974, I think it's not possible anymore to select some indicators of a composite contributor for a health group ; we can include/exclude the whole composite contributor but not part of it.

Composite contributors are used by default by various kind of ressouces in Spring boot : databases, jms connexion, mail serveur etc...

Concrete use case: on an application with two datasources, i would like to be able to include the health indicator of one datasource in readiness group without including the health indicator of the other datasource.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 20, 2020
@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Jan 15, 2021
@philwebb philwebb added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Feb 22, 2021
@philwebb philwebb added this to the 2.5.x milestone Feb 22, 2021
@philwebb philwebb modified the milestones: 2.5.x, 2.6.x Mar 19, 2021
@mbhave mbhave self-assigned this Jul 27, 2021
@mbhave mbhave added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jul 27, 2021
@bclozel
Copy link
Member

bclozel commented Jul 28, 2021

We've discussed this and we'd like to understand the problem space a bit more:

  • was this ever supported and how is this failing currently? We want to investigate and figure out why this component is not found.
  • using a composite indicator doesn't give any guarantee about the presence or the names of its components; we're not sure that decomposing a composite and address one of its components directly is a feature we'd like to promote.

@christophejan
Copy link
Contributor Author

Thank you @bclozel.

First, regarding the points you raise :

  1. It's just that Health group with composite contributor results in 404 #19974 and it's closing commit (d485708) seems to show it was supported.

    In this issue philsttr says :

    If I change management.endpoint.health.group.foo.include to foo,foo-A,foo-B, the foo group will work. However, I cannot statically configure the individual indicator ids, since they will be dynamically created.

    (foo is a composite contributor containing foo-A and foor-B health indicators)

    The closing commit (d485708) seems to show as well that, before this fix, indicators nested in a composite contributor were filtered through group.isMember(name) in getAggregateHealth method of HealthEndpointSupport. The commit add a isNested boolean parameter that bypass filtering on nested indicators so they are always all included.

    With current Spring Boot version, i don't see an easy way to include/exclude only a part of a composite contributor.

  2. I don't really understand this point.

    I don't think there more guarantee on a the presence or the names with basic health indicators. They depends on the applications dependencies, the beans declared in the application, the presence of custom health indicators etc...

    Composite contributor are also already decomposed by Spring Boot health actuator when management.endpoint.health.show-components property is set to always :

    • For example, when you query /actuator/health with two configured datasource, it will return something like :
      {
      	"status": "UP",
      	"components": {
      		"db": {
      			"status": "UP",
      			"components": {
      				"firstDataSource": {
      					"status": "UP"
      				},
      				"secondDataSource": {
      					"status": "UP"
      				}
      			}
      		}
      	}
      }
      
    • It is even possible to query only one of the nested indicator. For example, /actuator/health/db/firstDataSource will return in that case : {"status":"UP"}

Additionally, please consider the following points :

  • spring-boot-actuator-autoconfigure create itself composite contributors for many kind of resources. There is currently 14 classes extending CompositeHealthContributorConfiguration in addition to DataSourceHealthContributorAutoConfiguration that call CompositeHealthContributor.fromMap(Map<String, V>, Function<V, ? extends HealthContributor>) directly. I've not check them all but I think they always follow the same pattern. They create a composite contributor during Spring startup from a set of bean of a specific type, using the bean names as id for nested indicators. The set of nested indicators and their id is stable and perfectly known by the teams that manage the application. A basic example is the one provide by Configure Two DataSources in Spring Boot Reference Documentation.
  • The basic data of health actuator is health indicator. Composite contributor nodes (and root node) just do aggregation ; like a hierarchy on an OLAP cube ; a view on these data. I think the goal of Health Groups is to be able to get the status of a subset of health indicators ; not a subset of aggregation/categories/kinds/types.

I didn't talk about how include/exclude part of a composite contributor in a health group could be implemented. There is many options with different benefit and drawback :

  • to keep going with what was initially done
  • use health indicator path and path matcher (ant or regexp) (path concept was already present on some methods of HealthEndpointSupport
  • introduce new application properties
  • getting rid of composite contributor by just associating a health indicator to a path through a health indicator attribute
  • etc...

I hope this will help.

Best regards

@mbhave mbhave changed the title Restore the ability to include/exclude part of a composite contributor in a health group Add ability to include/exclude part of a composite contributor in a health group Aug 16, 2021
@mbhave
Copy link
Contributor

mbhave commented Aug 17, 2021

Thanks @christophejan. Being able to include/exclude part of a composite was not possible even prior to #19974. The group.isMember check would have prevented that from happening. The reason it worked in this comment

If I change management.endpoint.health.group.foo.include to foo,foo-A,foo-B, the foo group will work.

was because both foo and foo-A were specified as members. However, it would not have worked if just foo-A was a configured as a member.

That being said, I discussed this with @philwebb and we think it is an enhancement worth adding. Since there is no guarantee that components within multiple composites cannot have the same name, we need a way of qualifying them somehow. We can do this by specifying foo/foo-A to include/exclude a contributor with the name foo-A that is part of the composite indicator foo.

@mbhave mbhave removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Aug 17, 2021
@mbhave mbhave closed this as completed in 8fd9eb7 Aug 18, 2021
@mbhave mbhave modified the milestones: 2.6.x, 2.6.0-M2 Aug 18, 2021
@christophejan
Copy link
Contributor Author

Great !

Thanks a lot to everyone

Best regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants