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

Move health indicator logging from indicators to a central location #43589

Open
philwebb opened this issue Dec 20, 2024 · 2 comments
Open

Move health indicator logging from indicators to a central location #43589

philwebb opened this issue Dec 20, 2024 · 2 comments
Labels
status: pending-design-work Needs design work before any code can be developed type: enhancement A general enhancement
Milestone

Comments

@philwebb
Copy link
Member

See #43492 for background.

We'd like to centralize the logging when a health indicator fails so that it can be changed easily. This will involve:

  • Removing the existing logging from AbstractHealthIndicator and AbstractReactiveHealthIndicator (the logExceptionIfPresent method).
  • Updating Health so that the exception can be obtained.
  • Centralizing logging to the code the calls the health indicators.
@josephabonasara

This comment was marked as outdated.

@philwebb philwebb added the status: pending-design-work Needs design work before any code can be developed label Dec 23, 2024
@marcusvoltolim
Copy link

marcusvoltolim commented Dec 27, 2024

My solution to this might help with brainstorming:

import java.util.List;
import java.util.Map;
import java.util.Optional;
import com.fasterxml.jackson.annotation.JsonGetter;
import com.github.seratch.jslack.api.model.Field;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.reflect.FieldUtils;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.actuate.autoconfigure.health.HealthEndpointProperties;
import org.springframework.boot.actuate.health.Health;
import org.springframework.boot.actuate.health.HealthComponent;
import org.springframework.boot.actuate.health.HealthEndpointGroups;
import org.springframework.boot.actuate.health.ReactiveHealthContributor;
import org.springframework.boot.actuate.health.ReactiveHealthContributorRegistry;
import org.springframework.boot.actuate.health.ReactiveHealthEndpointWebExtension;
import org.springframework.boot.actuate.health.Status;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;

import static org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication.Type.REACTIVE;

@Slf4j
@ConditionalOnProperty("management.health.custom-extension.enabled")
@ConditionalOnWebApplication(type = REACTIVE)
@Component
public final class CustomReactiveHealthEndpointWebExtension extends ReactiveHealthEndpointWebExtension {

    private final Optional<MultiLogger> splunkLoggerOpt;
    private final String slackUrl;
    private final String hostname;

    CustomReactiveHealthEndpointWebExtension(final ReactiveHealthContributorRegistry registry,
                                              final HealthEndpointGroups groups,
                                              final HealthEndpointProperties properties,
                                              final Optional<MultiLogger> splunkLoggerOpt,
                                              final @Value("${management.health.custom-extension.slack-url:}") String slackUrl,
                                              final @Value("${management.health.custom-extension.hostname:}") String hostname) {
        super(registry, groups, properties.getLogging().getSlowIndicatorThreshold());
        this.splunkLoggerOpt = splunkLoggerOpt;
        this.slackUrl = slackUrl;
        this.hostname = hostname;
    }

    @Override
    protected Mono<? extends HealthComponent> getHealth(final ReactiveHealthContributor contributor, final boolean includeDetails) {
        return super.getHealth(contributor, includeDetails)
            .doOnNext(healthComponent -> {
                if (healthComponent.getStatus() != Status.UP) {
                    var healthErrorLog = new HealthErrorLog(contributor, healthComponent);

                    splunkLoggerOpt.ifPresentOrElse(
                        multiLogger -> multiLogger.error(healthErrorLog, null),
                        () -> log.error("Health indicator failed, details: {}", healthErrorLog)
                    );
                    if (!slackUrl.isBlank()) {
                        SlackUtil.send(slackUrl, SLACK_ALERT_COLOR, "HEALTH ALERT - " + hostname, "Health indicator failed!", healthErrorLog.buildSlackFields());
                    }
                }
            });
    }

    private record HealthErrorLog(@JsonGetter("_type") String type, String name, String status, Map<?, ?> details) {

        private static final String LOG_TYPE = "health-error";

        private HealthErrorLog(final ReactiveHealthContributor contributor, final HealthComponent healthComponent) {
            this(LOG_TYPE, getHealthIndicatorName(contributor), healthComponent.getStatus().toString(), healthComponent instanceof Health health ? health.getDetails() : Map.of());
        }

        public List<Field> buildSlackFields() {
            return List.of(
                new Field("name", name, true),
                new Field("status", status, true),
                new Field("details", details.toString(), false)
            );
        }

        /**
         * Workaround to get name from package-private class
         *
         * @see org.springframework.boot.actuate.health.HealthIndicatorReactiveAdapter
         * @see org.springframework.boot.actuate.health.CompositeHealthContributorReactiveAdapter
         */
        private static String getHealthIndicatorName(final ReactiveHealthContributor contributor) {
            try {
                var delegate = FieldUtils.readField(contributor, "delegate", true);
                return delegate.getClass().getSimpleName();
            } catch (Exception ignored) {
                return contributor.getClass().getSimpleName();
            }
        }

    }

}

Something that would help a lot and would be very simple to change, is to pass the name parameter in the method below, this would make it easier to overwrite and avoid the use of Reflection to obtain the name of the HealthIndicatorReactiveAdapter

org.springframework.boot.actuate.health.HealthEndpointSupport

protected abstract T getHealth(C contributor, boolean includeDetails);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: pending-design-work Needs design work before any code can be developed type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants