From ec38412b2aacea93d116a48a0c8fdb3485894ca9 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 23 Mar 2023 09:18:03 -0400 Subject: [PATCH 1/2] Json parser string length metrics are tagged by format This allows us to differentiate between JSON and SMILE mappers. --- .../java/serialization/InstrumentedJsonFactory.java | 2 +- .../java/serialization/InstrumentedSmileFactory.java | 4 ++-- .../java/serialization/ParserInstrumentation.java | 4 ++-- .../src/main/metrics/jackson-registry-metrics.yml | 2 ++ .../conjure/java/serialization/ObjectMappersTest.java | 10 ++++++---- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/InstrumentedJsonFactory.java b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/InstrumentedJsonFactory.java index 90d1850da..78e712f01 100644 --- a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/InstrumentedJsonFactory.java +++ b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/InstrumentedJsonFactory.java @@ -53,7 +53,7 @@ final class InstrumentedJsonFactory extends JsonFactory { private final ParserInstrumentation instrumentation; InstrumentedJsonFactory() { - this.instrumentation = new ParserInstrumentation(); + this.instrumentation = new ParserInstrumentation(getFormatName()); } private InstrumentedJsonFactory(JsonFactoryBuilder builder, ParserInstrumentation instrumentation) { diff --git a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/InstrumentedSmileFactory.java b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/InstrumentedSmileFactory.java index 23605e05d..4cba64cb0 100644 --- a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/InstrumentedSmileFactory.java +++ b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/InstrumentedSmileFactory.java @@ -34,11 +34,11 @@ final class InstrumentedSmileFactory extends SmileFactory { private final ParserInstrumentation instrumentation; InstrumentedSmileFactory() { - instrumentation = new ParserInstrumentation(); + instrumentation = new ParserInstrumentation(getFormatName()); } private InstrumentedSmileFactory(SmileFactoryBuilder builder) { - this(builder, new ParserInstrumentation()); + this(builder, new ParserInstrumentation(FORMAT_NAME_SMILE)); } private InstrumentedSmileFactory(SmileFactoryBuilder builder, ParserInstrumentation instrumentation) { diff --git a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ParserInstrumentation.java b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ParserInstrumentation.java index d43e935e3..f3824c104 100644 --- a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ParserInstrumentation.java +++ b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ParserInstrumentation.java @@ -35,10 +35,10 @@ final class ParserInstrumentation { // Using the shared metric registry singleton to avoid API churn in methods that use this instrumentation. @SuppressWarnings("deprecation") - ParserInstrumentation() { + ParserInstrumentation(String format) { creationStackTrace = new SafeRuntimeException("Stream factory created here"); parsedStringLength = JsonParserMetrics.of(SharedTaggedMetricRegistries.getSingleton()) - .stringLength(); + .stringLength(format); } /** Returns the input, recording length of the value. */ diff --git a/conjure-java-jackson-serialization/src/main/metrics/jackson-registry-metrics.yml b/conjure-java-jackson-serialization/src/main/metrics/jackson-registry-metrics.yml index 8b3bf728b..83c56308b 100644 --- a/conjure-java-jackson-serialization/src/main/metrics/jackson-registry-metrics.yml +++ b/conjure-java-jackson-serialization/src/main/metrics/jackson-registry-metrics.yml @@ -7,4 +7,6 @@ namespaces: metrics: string.length: type: histogram + tags: + - format docs: Histogram describing the length of strings parsed from input. diff --git a/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java b/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java index 4188db527..1f78c77b5 100644 --- a/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java +++ b/conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java @@ -21,12 +21,14 @@ import com.codahale.metrics.Histogram; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.exc.InputCoercionException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.exc.InvalidFormatException; import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.dataformat.smile.SmileFactory; import com.palantir.logsafe.Preconditions; import com.palantir.tritium.metrics.registry.SharedTaggedMetricRegistries; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; @@ -319,7 +321,7 @@ public void testStringMetrics_json() throws IOException { TaggedMetricRegistry registry = SharedTaggedMetricRegistries.getSingleton(); // Unregister all metrics registry.forEachMetric((name, _value) -> registry.remove(name)); - Histogram stringLength = JsonParserMetrics.of(registry).stringLength(); + Histogram stringLength = JsonParserMetrics.of(registry).stringLength(JsonFactory.FORMAT_NAME_JSON); assertThat(stringLength.getSnapshot().size()).isZero(); // Length must exceed the minimum threshold for metrics (64 characters) String expected = "Hello, World!".repeat(10); @@ -334,7 +336,7 @@ public void testStringMetricsNotRecordedWhenValuesAreSmall_json() throws IOExcep TaggedMetricRegistry registry = SharedTaggedMetricRegistries.getSingleton(); // Unregister all metrics registry.forEachMetric((name, _value) -> registry.remove(name)); - Histogram stringLength = JsonParserMetrics.of(registry).stringLength(); + Histogram stringLength = JsonParserMetrics.of(registry).stringLength(JsonFactory.FORMAT_NAME_JSON); assertThat(stringLength.getSnapshot().size()).isZero(); String expected = "Hello, World!"; String value = ObjectMappers.newServerJsonMapper().readValue("\"" + expected + "\"", String.class); @@ -347,7 +349,7 @@ public void testStringMetrics_smile() throws IOException { TaggedMetricRegistry registry = SharedTaggedMetricRegistries.getSingleton(); // Unregister all metrics registry.forEachMetric((name, _value) -> registry.remove(name)); - Histogram stringLength = JsonParserMetrics.of(registry).stringLength(); + Histogram stringLength = JsonParserMetrics.of(registry).stringLength(SmileFactory.FORMAT_NAME_SMILE); assertThat(stringLength.getSnapshot().size()).isZero(); // Length must exceed the minimum threshold for metrics (64 characters) String expected = "Hello, World!".repeat(10); @@ -363,7 +365,7 @@ public void testStringMetricsNotRecordedWhenValuesAreSmall_smile() throws IOExce TaggedMetricRegistry registry = SharedTaggedMetricRegistries.getSingleton(); // Unregister all metrics registry.forEachMetric((name, _value) -> registry.remove(name)); - Histogram stringLength = JsonParserMetrics.of(registry).stringLength(); + Histogram stringLength = JsonParserMetrics.of(registry).stringLength(SmileFactory.FORMAT_NAME_SMILE); assertThat(stringLength.getSnapshot().size()).isZero(); String expected = "Hello, World!"; String value = ObjectMappers.newServerSmileMapper() From 9b657db3aaf101ce71aa0ef6fa2f89640c4efd17 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Thu, 23 Mar 2023 13:26:45 +0000 Subject: [PATCH 2/2] Add generated changelog entries --- changelog/@unreleased/pr-2571.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-2571.v2.yml diff --git a/changelog/@unreleased/pr-2571.v2.yml b/changelog/@unreleased/pr-2571.v2.yml new file mode 100644 index 000000000..a364f7175 --- /dev/null +++ b/changelog/@unreleased/pr-2571.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Json parser string length metrics are tagged by format + links: + - https://github.com/palantir/conjure-java-runtime/pull/2571