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

Cleanup logging #33

Merged
merged 1 commit into from
Jul 23, 2024
Merged

Cleanup logging #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/io/strimzi/kafka/metrics/DataPointSnapshotBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
*/
public class DataPointSnapshotBuilder {

private static final Logger LOG = LoggerFactory.getLogger(DataPointSnapshotBuilder.class.getName());
private static final Logger LOG = LoggerFactory.getLogger(DataPointSnapshotBuilder.class);

/**
* Create a datapoint for a {@link InfoSnapshot} metric
Expand Down
8 changes: 3 additions & 5 deletions src/main/java/io/strimzi/kafka/metrics/KafkaMetricsCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
*/
public class KafkaMetricsCollector implements MultiCollector {

private static final Logger LOG = LoggerFactory.getLogger(KafkaMetricsCollector.class.getName());
private static final Logger LOG = LoggerFactory.getLogger(KafkaMetricsCollector.class);

private final Map<MetricName, KafkaMetric> metrics;
private final PrometheusMetricsReporterConfig config;
Expand Down Expand Up @@ -87,16 +87,14 @@ public MetricSnapshots collect() {
for (Map.Entry<MetricName, KafkaMetric> entry : metrics.entrySet()) {
MetricName metricName = entry.getKey();
KafkaMetric kafkaMetric = entry.getValue();
LOG.trace("Collecting Kafka metric {}", metricName);

String prometheusMetricName = metricName(metricName);
// TODO Filtering should take labels into account
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed in #5, we decided to not implement that feature now. So removing the TODO. Same in YammerMetricsCollector below.

if (!config.isAllowed(prometheusMetricName)) {
LOG.info("Kafka metric {} is not allowed", prometheusMetricName);
LOG.trace("Ignoring metric {} as it does not match the allowlist", prometheusMetricName);
continue;
}
LOG.info("Kafka metric {} is allowed", prometheusMetricName);
Labels labels = labelsFromTags(metricName.tags(), metricName.name());
LOG.debug("Collecting metric {} with the following labels: {}", prometheusMetricName, labels);

Object valueObj = kafkaMetric.metricValue();
if (valueObj instanceof Number) {
Expand Down
7 changes: 2 additions & 5 deletions src/main/java/io/strimzi/kafka/metrics/KafkaPrometheusMetricsReporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
*/
public class KafkaPrometheusMetricsReporter implements MetricsReporter {

private static final Logger LOG = LoggerFactory.getLogger(KafkaPrometheusMetricsReporter.class.getName());
private static final Logger LOG = LoggerFactory.getLogger(KafkaPrometheusMetricsReporter.class);

private final PrometheusRegistry registry;
@SuppressFBWarnings({"UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR"}) // This field is initialized in the configure method
Expand All @@ -55,6 +55,7 @@ public void configure(Map<String, ?> map) {
// Add JVM metrics
JvmMetrics.builder().register(registry);
httpServer = config.startHttpServer();
LOG.debug("KafkaPrometheusMetricsReporter configured with {}", config);
}

@Override
Expand All @@ -67,20 +68,17 @@ public void init(List<KafkaMetric> metrics) {

@Override
public void metricChange(KafkaMetric metric) {
LOG.info("Kafka metricChange {}", metric.metricName());
kafkaMetricsCollector.addMetric(metric);
}

@Override
public void metricRemoval(KafkaMetric metric) {
LOG.info("Kafka metricRemoval {}", metric.metricName());
kafkaMetricsCollector.removeMetric(metric);
}

@Override
public void close() {
registry.unregister(kafkaMetricsCollector);
LOG.info("Closing the HTTP server");
}

@Override
Expand All @@ -98,7 +96,6 @@ public Set<String> reconfigurableConfigs() {

@Override
public void contextChange(MetricsContext metricsContext) {
LOG.info("Kafka contextChange with {}", metricsContext.contextLabels());
String prefix = metricsContext.contextLabels().get(MetricsContext.NAMESPACE);
kafkaMetricsCollector.setPrefix(prefix);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/io/strimzi/kafka/metrics/PrometheusMetricsReporterConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
*/
public class PrometheusMetricsReporterConfig extends AbstractConfig {

private static final Logger LOG = LoggerFactory.getLogger(PrometheusMetricsReporterConfig.class.getName());
private static final Logger LOG = LoggerFactory.getLogger(PrometheusMetricsReporterConfig.class);
private static final String CONFIG_PREFIX = "prometheus.metrics.reporter.";

/**
Expand Down
18 changes: 6 additions & 12 deletions src/main/java/io/strimzi/kafka/metrics/YammerMetricsCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
@SuppressWarnings("ClassFanOutComplexity")
public class YammerMetricsCollector implements MultiCollector {

private static final Logger LOG = LoggerFactory.getLogger(YammerMetricsCollector.class.getName());
private static final Logger LOG = LoggerFactory.getLogger(YammerMetricsCollector.class);
private static final List<Double> QUANTILES = Arrays.asList(0.50, 0.75, 0.95, 0.98, 0.99, 0.999);

private final List<MetricsRegistry> registries;
Expand Down Expand Up @@ -76,17 +76,14 @@ public MetricSnapshots collect() {
for (Map.Entry<MetricName, Metric> entry : registry.allMetrics().entrySet()) {
MetricName metricName = entry.getKey();
Metric metric = entry.getValue();
LOG.trace("Collecting Yammer metric {}", metricName);

String prometheusMetricName = metricName(metricName);
// TODO Filtering should take labels into account
if (!config.isAllowed(prometheusMetricName)) {
LOG.info("Yammer metric {} is not allowed", prometheusMetricName);
LOG.trace("Ignoring metric {} as it does not match the allowlist", prometheusMetricName);
continue;
}
LOG.info("Yammer metric {} is allowed", prometheusMetricName);
Labels labels = labelsFromScope(metricName.getScope(), prometheusMetricName);
LOG.info("labels {}", labels);
LOG.debug("Collecting metric {} with the following labels: {}", prometheusMetricName, labels);

if (metric instanceof Counter) {
Counter counter = (Counter) metric;
Expand Down Expand Up @@ -115,7 +112,7 @@ public MetricSnapshots collect() {
CounterSnapshot.Builder builder = counterBuilders.computeIfAbsent(prometheusMetricName, k -> CounterSnapshot.builder().name(prometheusMetricName));
builder.dataPoint(DataPointSnapshotBuilder.counterDataPoint(labels, meter.count()));
} else {
LOG.error("The metric {} has an unexpected type.", metric.getClass().getName());
LOG.error("The metric {} has an unexpected type: {}", prometheusMetricName, metric.getClass().getName());
}
}
}
Expand All @@ -136,13 +133,11 @@ public MetricSnapshots collect() {
}

private static String metricName(MetricName metricName) {
String metricNameStr = PrometheusNaming.sanitizeMetricName(
return PrometheusNaming.sanitizeMetricName(
"kafka_server_" +
metricName.getGroup() + '_' +
metricName.getType() + '_' +
metricName.getName()).toLowerCase(Locale.ROOT);
LOG.info("metricName group {}, type {}, name {} converted into {}", metricName.getGroup(), metricName.getType(), metricName.getName(), metricNameStr);
return metricNameStr;
}

static Labels labelsFromScope(String scope, String metricName) {
Expand All @@ -156,8 +151,7 @@ static Labels labelsFromScope(String scope, String metricName) {
if (labelNames.add(newLabelName)) {
builder.label(newLabelName, parts[i + 1]);
} else {
String value = parts[i + 1];
LOG.warn("Ignoring duplicate label key: {} with value: {} from metric: {} ", newLabelName, value, metricName);
LOG.warn("Ignoring duplicate label key: {} with value: {} from metric: {} ", newLabelName, parts[i + 1], metricName);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/strimzi/kafka/metrics/YammerPrometheusMetricsReporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
public class YammerPrometheusMetricsReporter implements KafkaMetricsReporter {

private static final Logger LOG = LoggerFactory.getLogger(YammerPrometheusMetricsReporter.class.getName());
private static final Logger LOG = LoggerFactory.getLogger(YammerPrometheusMetricsReporter.class);

private final PrometheusRegistry registry;

Expand All @@ -33,9 +33,9 @@ public YammerPrometheusMetricsReporter() {

@Override
public void init(VerifiableProperties props) {
LOG.info(">>> in init() yammer");
PrometheusMetricsReporterConfig config = new PrometheusMetricsReporterConfig(props.props(), registry);
registry.register(new YammerMetricsCollector(config));
LOG.debug("YammerPrometheusMetricsReporter configured with {}", config);
}

}