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
Improved subscription health problem indicators #972
Conversation
piotrrzysko
commented
Jan 18, 2019
- Removed SlowIndicator
- Added parameters to enable/disable indicators
- Fixed indicators to respect delivery type of subscription
@@ -26,7 +27,7 @@ public SubscriptionMetrics(@JsonProperty("delivered") long delivered, @JsonPrope | |||
@JsonProperty("otherErrors") String otherErrors, @JsonProperty("codes2xx") String codes2xx, | |||
@JsonProperty("codes4xx") String codes4xx, @JsonProperty("codes5xx") String codes5xx, | |||
@JsonProperty("Subscription") Subscription.State state, @JsonProperty("rate") String rate, | |||
@JsonProperty("throughput") String throughput) { | |||
@JsonProperty("throughput") String throughput, @JsonProperty("batchRate") String batchRate) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split into a separate lines please 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
public SubscriptionHealthContext(Subscription subscription, TopicMetrics topicMetrics, SubscriptionMetrics subscriptionMetrics) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IDEA's suggestion? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup :)
import pl.allegro.tech.hermes.management.domain.subscription.health.problem.SlowIndicator | ||
import pl.allegro.tech.hermes.management.domain.subscription.health.problem.TimingOutIndicator | ||
import pl.allegro.tech.hermes.management.domain.subscription.health.problem.UnreachableIndicator | ||
import pl.allegro.tech.hermes.management.domain.subscription.health.problem.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't do this 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
import static pl.allegro.tech.hermes.api.SubscriptionHealthProblem.slow | ||
import static pl.allegro.tech.hermes.api.SubscriptionHealthProblem.timingOut | ||
import static pl.allegro.tech.hermes.api.SubscriptionHealthProblem.unreachable | ||
import static pl.allegro.tech.hermes.api.SubscriptionHealthProblem.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
if (healthProperties.isLaggingIndicatorEnabled()) { | ||
return new LaggingIndicator(healthProperties.getMaxLagInSeconds()); | ||
} | ||
return new DisabledIndicator(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we store a single static instance of the DisabledIndicator
and return everywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍