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

fix empty histograms from micrometer being sent to the server #3304

Merged
merged 3 commits into from Aug 31, 2023
Merged

fix empty histograms from micrometer being sent to the server #3304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db475c8
fix empty histograms from micrometer being sent to the server
jackshirazi Aug 31, 2023
8c83ece
update changelog
jackshirazi Aug 31, 2023
4e76407
Merge branch 'elastic:main' into fix-empty-histos-from-micrometer
jackshirazi Aug 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.asciidoc
View file
Open in desktop
Expand Up @@ -35,7 +35,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
===== Bug fixes
* Prevent bad serialization in edge cases for span compression - {pull}3293[#3293]
* Allow overriding of transaction type for Servlet-API transactions - {pull}3226[#3226]
* Fix micrometer histogram serialization - {pull}3290[#3290]
* Fix micrometer histogram serialization - {pull}3290[#3290], {pull}3304[#3304]
* Fix transactions not being correctly handled in certain edge cases - {pull}3294[#3294]

[float]
Expand Down
8 changes: 5 additions & 3 deletions ...ugin/src/main/java/co/elastic/apm/agent/micrometer/MicrometerMeterRegistrySerializer.java
View file
Open in desktop
Expand Up @@ -201,7 +201,7 @@ private static boolean serializeTimer(JsonWriter jw, HistogramSnapshot histogram
serializeValue(id, ".count", count, jw, replaceBuilder, dedotMetricName);
jw.writeByte(JsonWriter.COMMA);
serializeValue(id, ".sum.us", totalTime, jw, replaceBuilder, dedotMetricName);
if (histogramSnapshot != null) {
if (histogramSnapshot != null && histogramSnapshot.histogramCounts().length > 0) {
jw.writeByte(JsonWriter.COMMA);
serializeHistogram(id, histogramSnapshot, jw, replaceBuilder, dedotMetricName);
}
Expand Down Expand Up @@ -229,8 +229,10 @@ private static boolean serializeDistributionSummary(JsonWriter jw, HistogramSnap
serializeValue(id, ".count", count, jw, replaceBuilder, dedotMetricName);
jw.writeByte(JsonWriter.COMMA);
serializeValue(id, ".sum", totalAmount, jw, replaceBuilder, dedotMetricName);
jw.writeByte(JsonWriter.COMMA);
serializeHistogram(id, histogramSnapshot, jw, replaceBuilder, dedotMetricName);
if (histogramSnapshot != null && histogramSnapshot.histogramCounts().length > 0) {
jw.writeByte(JsonWriter.COMMA);
serializeHistogram(id, histogramSnapshot, jw, replaceBuilder, dedotMetricName);
}
return true;
}
return hasValue;
Expand Down
63 changes: 52 additions & 11 deletions .../src/test/java/co/elastic/apm/agent/micrometer/MicrometerMeterRegistrySerializerTest.java
View file
Open in desktop
Expand Up @@ -80,6 +80,11 @@ void serializeDistributionSummary() {
serializeOneMeter(new TestSummary());
}

@Test
void serializeDistributionSummaryWithNoValues() {
serializeOneMeter(new TestSummary(false));
}

@Test
void serializeGauge() {
serializeOneMeter(new TestGauge());
Expand Down Expand Up @@ -201,9 +206,18 @@ protected static void checkPathHasValue(JsonNode jsonNode, String[] path, int va
}

protected static JsonNode getPathNode(JsonNode jsonNode, String[] path) {
return getPathNode(jsonNode, path, false);
}
protected static JsonNode getPathNode(JsonNode jsonNode, String[] path, boolean isNull) {
assertThat(jsonNode).isNotNull();
for (String element: path) {
jsonNode = jsonNode.get(element);
for (int i = 0; i < path.length-1; i++) {
jsonNode = jsonNode.get(path[i]);
assertThat(jsonNode).isNotNull();
}
jsonNode = jsonNode.get(path[path.length-1]);
if (isNull) {
assertThat(jsonNode).isNull();
} else {
assertThat(jsonNode).isNotNull();
}
return jsonNode;
Expand Down Expand Up @@ -261,26 +275,42 @@ public void checkSerialization(JsonNode jsonNode) {
}

static class TestSummary extends TestMeter {
private final boolean setSLOs;
int sum = 0;
int count = 0;
int under5Count = 0;
int from5To50Count = 0;
int from50to95Count = 0;
int[] values = new int[]{22, 55, 66, 98};

public TestSummary () {
this(true);
}
public TestSummary (boolean setSLOs) {
super();
this.setSLOs = setSLOs;
}
@Override
String meternameExtension() {
return ".count";
}

@Override
public void addToMeterRegistry(MeterRegistry registry) {
meter = DistributionSummary
.builder(metername())
.distributionStatisticBufferLength(20)
.serviceLevelObjectives(5, 50, 95)
.publishPercentileHistogram()
.register(registry);
if (setSLOs) {
meter = DistributionSummary
.builder(metername())
.distributionStatisticBufferLength(20)
.serviceLevelObjectives(5, 50, 95)
.publishPercentileHistogram()
.register(registry);
} else {
meter = DistributionSummary
.builder(metername())
.distributionStatisticBufferLength(20)
.publishPercentileHistogram()
.register(registry);
}
}

@Override
Expand All @@ -303,9 +333,20 @@ public void populateValues() {
public void checkSerialization(JsonNode jsonNode) {
checkPathHasValue(jsonNode, path(), count);
checkPathHasValue(jsonNode, path(".sum"), sum);
String[] path = path(".histogram");
path[3] = "values";
JsonNode histoNode1 = getPathNode(jsonNode, path);
String[] temppath = path(".histogram");
String[] path;
if (!setSLOs) {
path = new String[temppath.length-1];
System.arraycopy(temppath, 0, path, 0, path.length);
} else {
path = temppath;
path[3] = "values";
}
JsonNode histoNode1 = getPathNode(jsonNode, path, !setSLOs);
if(!setSLOs) {
assertThat(histoNode1).isNull();
return;
}
assertThat(histoNode1.isArray()).isTrue();
assertThat(histoNode1.size()).isEqualTo(3); //the 3 bucket boundaries of the SLOs 5,50,95
assertThat(histoNode1.get(0).asDouble()).isEqualTo(5.0);
Expand Down