diff --git a/changelog/@unreleased/pr-731.v2.yml b/changelog/@unreleased/pr-731.v2.yml new file mode 100644 index 00000000..f5d97af0 --- /dev/null +++ b/changelog/@unreleased/pr-731.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Generated metric utilities require safe tag values + links: + - https://github.com/palantir/metric-schema/pull/731 diff --git a/metric-schema-java/src/integrationInput/java/com/palantir/test/MonitorsMetrics.java b/metric-schema-java/src/integrationInput/java/com/palantir/test/MonitorsMetrics.java index 1d83be4f..1b6b175f 100644 --- a/metric-schema-java/src/integrationInput/java/com/palantir/test/MonitorsMetrics.java +++ b/metric-schema-java/src/integrationInput/java/com/palantir/test/MonitorsMetrics.java @@ -3,9 +3,10 @@ import com.codahale.metrics.Meter; import com.google.errorprone.annotations.CheckReturnValue; import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; import com.palantir.tritium.metrics.registry.MetricName; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; -import java.util.Optional; +import java.util.Objects; /** * General web server metrics. @@ -13,9 +14,8 @@ public final class MonitorsMetrics { private static final String LIBRARY_NAME = "witchcraft"; - private static final String LIBRARY_VERSION = Optional.ofNullable( - MonitorsMetrics.class.getPackage().getImplementationVersion()) - .orElse("unknown"); + private static final String LIBRARY_VERSION = + Objects.requireNonNullElse(MonitorsMetrics.class.getPackage().getImplementationVersion(), "unknown"); private final TaggedMetricRegistry registry; @@ -39,7 +39,7 @@ public ProcessingBuilderResultStage processing() { * Measures more */ @CheckReturnValue - public Meter more(String type) { + public Meter more(@Safe String type) { return registry.meter(MetricName.builder() .safeName("monitors.more") .putSafeTags("type", type) @@ -96,17 +96,17 @@ public interface ProcessingBuilderResultStage { * The result of processing */ @CheckReturnValue - ProcessingBuilderTypeStage result(Processing_Result result); + ProcessingBuilderTypeStage result(@Safe Processing_Result result); } public interface ProcessingBuilderTypeStage { @CheckReturnValue - ProcessingBuilderOtherLocatorStage type(String type); + ProcessingBuilderOtherLocatorStage type(@Safe String type); } public interface ProcessingBuilderOtherLocatorStage { @CheckReturnValue - ProcessingBuildStage otherLocator(Processing_OtherLocator otherLocator); + ProcessingBuildStage otherLocator(@Safe Processing_OtherLocator otherLocator); } private final class ProcessingBuilder @@ -134,21 +134,21 @@ public Meter build() { } @Override - public ProcessingBuilder result(Processing_Result result) { + public ProcessingBuilder result(@Safe Processing_Result result) { Preconditions.checkState(this.result == null, "result is already set"); this.result = Preconditions.checkNotNull(result, "result is required"); return this; } @Override - public ProcessingBuilder type(String type) { + public ProcessingBuilder type(@Safe String type) { Preconditions.checkState(this.type == null, "type is already set"); this.type = Preconditions.checkNotNull(type, "type is required"); return this; } @Override - public ProcessingBuilder otherLocator(Processing_OtherLocator otherLocator) { + public ProcessingBuilder otherLocator(@Safe Processing_OtherLocator otherLocator) { Preconditions.checkState(this.otherLocator == null, "otherLocator is already set"); this.otherLocator = Preconditions.checkNotNull(otherLocator, "otherLocator is required"); return this; diff --git a/metric-schema-java/src/integrationInput/java/com/palantir/test/MyNamespaceMetrics.java b/metric-schema-java/src/integrationInput/java/com/palantir/test/MyNamespaceMetrics.java index 1cf82608..7ab7264a 100644 --- a/metric-schema-java/src/integrationInput/java/com/palantir/test/MyNamespaceMetrics.java +++ b/metric-schema-java/src/integrationInput/java/com/palantir/test/MyNamespaceMetrics.java @@ -4,9 +4,10 @@ import com.codahale.metrics.Histogram; import com.google.errorprone.annotations.CheckReturnValue; import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; import com.palantir.tritium.metrics.registry.MetricName; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; -import java.util.Optional; +import java.util.Objects; /** * General web server metrics. @@ -14,9 +15,8 @@ public final class MyNamespaceMetrics { private static final String LIBRARY_NAME = "witchcraft"; - private static final String LIBRARY_VERSION = Optional.ofNullable( - MyNamespaceMetrics.class.getPackage().getImplementationVersion()) - .orElse("unknown"); + private static final String LIBRARY_VERSION = + Objects.requireNonNullElse(MyNamespaceMetrics.class.getPackage().getImplementationVersion(), "unknown"); private final TaggedMetricRegistry registry; @@ -63,12 +63,12 @@ public interface ResponseSizeBuildStage { public interface ResponseSizeBuilderServiceNameStage { @CheckReturnValue - ResponseSizeBuilderEndpointStage serviceName(String serviceName); + ResponseSizeBuilderEndpointStage serviceName(@Safe String serviceName); } public interface ResponseSizeBuilderEndpointStage { @CheckReturnValue - ResponseSizeBuildStage endpoint(String endpoint); + ResponseSizeBuildStage endpoint(@Safe String endpoint); } private final class ResponseSizeBuilder @@ -89,14 +89,14 @@ public Histogram build() { } @Override - public ResponseSizeBuilder serviceName(String serviceName) { + public ResponseSizeBuilder serviceName(@Safe String serviceName) { Preconditions.checkState(this.serviceName == null, "service-name is already set"); this.serviceName = Preconditions.checkNotNull(serviceName, "service-name is required"); return this; } @Override - public ResponseSizeBuilder endpoint(String endpoint) { + public ResponseSizeBuilder endpoint(@Safe String endpoint) { Preconditions.checkState(this.endpoint == null, "endpoint is already set"); this.endpoint = Preconditions.checkNotNull(endpoint, "endpoint is required"); return this; diff --git a/metric-schema-java/src/integrationInput/java/com/palantir/test/ReservedConflictMetrics.java b/metric-schema-java/src/integrationInput/java/com/palantir/test/ReservedConflictMetrics.java index 25aac0de..a9729b59 100644 --- a/metric-schema-java/src/integrationInput/java/com/palantir/test/ReservedConflictMetrics.java +++ b/metric-schema-java/src/integrationInput/java/com/palantir/test/ReservedConflictMetrics.java @@ -5,9 +5,10 @@ import com.codahale.metrics.Meter; import com.google.errorprone.annotations.CheckReturnValue; import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; import com.palantir.tritium.metrics.registry.MetricName; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; -import java.util.Optional; +import java.util.Objects; /** * Tests that reserved words are escaped. @@ -15,9 +16,8 @@ public final class ReservedConflictMetrics { private static final String LIBRARY_NAME = "witchcraft"; - private static final String LIBRARY_VERSION = Optional.ofNullable( - ReservedConflictMetrics.class.getPackage().getImplementationVersion()) - .orElse("unknown"); + private static final String LIBRARY_VERSION = Objects.requireNonNullElse( + ReservedConflictMetrics.class.getPackage().getImplementationVersion(), "unknown"); private final TaggedMetricRegistry registry; @@ -41,7 +41,7 @@ public IntBuilderIntStage int_() { * Meter with a single tag. */ @CheckReturnValue - public Meter long_(String int_) { + public Meter long_(@Safe String int_) { return registry.meter(MetricName.builder() .safeName("reserved.conflict.long") .putSafeTags("int", int_) @@ -85,17 +85,17 @@ public interface IntBuildStage { public interface IntBuilderIntStage { @CheckReturnValue - IntBuilderRegistryStage int_(String int_); + IntBuilderRegistryStage int_(@Safe String int_); } public interface IntBuilderRegistryStage { @CheckReturnValue - IntBuilderLongStage registry_(String registry_); + IntBuilderLongStage registry_(@Safe String registry_); } public interface IntBuilderLongStage { @CheckReturnValue - IntBuildStage long_(String long_); + IntBuildStage long_(@Safe String long_); } private final class IntBuilder @@ -119,21 +119,21 @@ public Histogram build() { } @Override - public IntBuilder int_(String int_) { + public IntBuilder int_(@Safe String int_) { Preconditions.checkState(this.int_ == null, "int is already set"); this.int_ = Preconditions.checkNotNull(int_, "int is required"); return this; } @Override - public IntBuilder registry_(String registry_) { + public IntBuilder registry_(@Safe String registry_) { Preconditions.checkState(this.registry_ == null, "registry is already set"); this.registry_ = Preconditions.checkNotNull(registry_, "registry is required"); return this; } @Override - public IntBuilder long_(String long_) { + public IntBuilder long_(@Safe String long_) { Preconditions.checkState(this.long_ == null, "long is already set"); this.long_ = Preconditions.checkNotNull(long_, "long is required"); return this; @@ -148,7 +148,7 @@ public interface DoubleBuildStage { public interface DoubleBuilderIntStage { @CheckReturnValue - DoubleBuildStage int_(String int_); + DoubleBuildStage int_(@Safe String int_); } private final class DoubleBuilder implements DoubleBuilderIntStage, DoubleBuildStage { @@ -170,7 +170,7 @@ public MetricName buildMetricName() { } @Override - public DoubleBuilder int_(String int_) { + public DoubleBuilder int_(@Safe String int_) { Preconditions.checkState(this.int_ == null, "int is already set"); this.int_ = Preconditions.checkNotNull(int_, "int is required"); return this; diff --git a/metric-schema-java/src/integrationInput/java/com/palantir/test/ServerMetrics.java b/metric-schema-java/src/integrationInput/java/com/palantir/test/ServerMetrics.java index a866db84..172d44f1 100644 --- a/metric-schema-java/src/integrationInput/java/com/palantir/test/ServerMetrics.java +++ b/metric-schema-java/src/integrationInput/java/com/palantir/test/ServerMetrics.java @@ -4,9 +4,10 @@ import com.codahale.metrics.Histogram; import com.google.errorprone.annotations.CheckReturnValue; import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; import com.palantir.tritium.metrics.registry.MetricName; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; -import java.util.Optional; +import java.util.Objects; /** * General web server metrics. @@ -14,9 +15,8 @@ public final class ServerMetrics { private static final String LIBRARY_NAME = "witchcraft"; - private static final String LIBRARY_VERSION = Optional.ofNullable( - ServerMetrics.class.getPackage().getImplementationVersion()) - .orElse("unknown"); + private static final String LIBRARY_VERSION = + Objects.requireNonNullElse(ServerMetrics.class.getPackage().getImplementationVersion(), "unknown"); private final TaggedMetricRegistry registry; @@ -63,12 +63,12 @@ public interface ResponseSizeBuildStage { public interface ResponseSizeBuilderServiceNameStage { @CheckReturnValue - ResponseSizeBuilderEndpointStage serviceName(String serviceName); + ResponseSizeBuilderEndpointStage serviceName(@Safe String serviceName); } public interface ResponseSizeBuilderEndpointStage { @CheckReturnValue - ResponseSizeBuildStage endpoint(String endpoint); + ResponseSizeBuildStage endpoint(@Safe String endpoint); } private final class ResponseSizeBuilder @@ -89,14 +89,14 @@ public Histogram build() { } @Override - public ResponseSizeBuilder serviceName(String serviceName) { + public ResponseSizeBuilder serviceName(@Safe String serviceName) { Preconditions.checkState(this.serviceName == null, "service-name is already set"); this.serviceName = Preconditions.checkNotNull(serviceName, "service-name is required"); return this; } @Override - public ResponseSizeBuilder endpoint(String endpoint) { + public ResponseSizeBuilder endpoint(@Safe String endpoint) { Preconditions.checkState(this.endpoint == null, "endpoint is already set"); this.endpoint = Preconditions.checkNotNull(endpoint, "endpoint is required"); return this; diff --git a/metric-schema-java/src/integrationInput/java/com/palantir/test/VisibilityMetrics.java b/metric-schema-java/src/integrationInput/java/com/palantir/test/VisibilityMetrics.java index 9c29d4a3..f73261e2 100644 --- a/metric-schema-java/src/integrationInput/java/com/palantir/test/VisibilityMetrics.java +++ b/metric-schema-java/src/integrationInput/java/com/palantir/test/VisibilityMetrics.java @@ -4,9 +4,10 @@ import com.codahale.metrics.Gauge; import com.google.errorprone.annotations.CheckReturnValue; import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; import com.palantir.tritium.metrics.registry.MetricName; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; -import java.util.Optional; +import java.util.Objects; /** * Tests we respect javaVisibility @@ -14,9 +15,8 @@ final class VisibilityMetrics { private static final String LIBRARY_NAME = "witchcraft"; - private static final String LIBRARY_VERSION = Optional.ofNullable( - VisibilityMetrics.class.getPackage().getImplementationVersion()) - .orElse("unknown"); + private static final String LIBRARY_VERSION = + Objects.requireNonNullElse(VisibilityMetrics.class.getPackage().getImplementationVersion(), "unknown"); private final TaggedMetricRegistry registry; @@ -24,7 +24,7 @@ private VisibilityMetrics(TaggedMetricRegistry registry) { this.registry = registry; } - public static VisibilityMetrics of(TaggedMetricRegistry registry) { + static VisibilityMetrics of(TaggedMetricRegistry registry) { return new VisibilityMetrics(Preconditions.checkNotNull(registry, "TaggedMetricRegistry")); } @@ -61,12 +61,12 @@ interface ComplexBuildStage { interface ComplexBuilderFooStage { @CheckReturnValue - ComplexBuilderBarStage foo(String foo); + ComplexBuilderBarStage foo(@Safe String foo); } interface ComplexBuilderBarStage { @CheckReturnValue - ComplexBuildStage bar(String bar); + ComplexBuildStage bar(@Safe String bar); } private final class ComplexBuilder implements ComplexBuilderFooStage, ComplexBuilderBarStage, ComplexBuildStage { @@ -91,14 +91,14 @@ public MetricName buildMetricName() { } @Override - public ComplexBuilder foo(String foo) { + public ComplexBuilder foo(@Safe String foo) { Preconditions.checkState(this.foo == null, "foo is already set"); this.foo = Preconditions.checkNotNull(foo, "foo is required"); return this; } @Override - public ComplexBuilder bar(String bar) { + public ComplexBuilder bar(@Safe String bar) { Preconditions.checkState(this.bar == null, "bar is already set"); this.bar = Preconditions.checkNotNull(bar, "bar is required"); return this; diff --git a/metric-schema-java/src/main/java/com/palantir/metric/schema/UtilityGenerator.java b/metric-schema-java/src/main/java/com/palantir/metric/schema/UtilityGenerator.java index 35765fed..927ac2c1 100644 --- a/metric-schema-java/src/main/java/com/palantir/metric/schema/UtilityGenerator.java +++ b/metric-schema-java/src/main/java/com/palantir/metric/schema/UtilityGenerator.java @@ -21,6 +21,7 @@ import com.google.common.collect.Iterables; import com.google.errorprone.annotations.CheckReturnValue; import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; import com.palantir.tritium.metrics.registry.MetricName; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import com.squareup.javapoet.ClassName; @@ -33,6 +34,7 @@ import com.squareup.javapoet.TypeSpec; import com.squareup.javapoet.WildcardTypeName; import java.util.List; +import java.util.Objects; import java.util.Optional; import javax.lang.model.element.Modifier; @@ -50,7 +52,7 @@ static JavaFile generateUtilityClass( .addModifiers(visibility.apply(Modifier.FINAL)) .addJavadoc(Javadoc.render(metrics.getDocs())) .addMethod(MethodSpec.methodBuilder(ReservedNames.FACTORY_METHOD) - .addModifiers(Modifier.PUBLIC, Modifier.STATIC) + .addModifiers(visibility.apply(Modifier.STATIC)) .addParameter(TaggedMetricRegistry.class, ReservedNames.REGISTRY_NAME) .addStatement( "return new $T($T.checkNotNull(registry, \"TaggedMetricRegistry\"))", @@ -71,8 +73,8 @@ static JavaFile generateUtilityClass( Modifier.STATIC, Modifier.FINAL) .initializer( - "$T.ofNullable($T.class.getPackage().getImplementationVersion()).orElse(\"unknown\")", - Optional.class, + "$T.requireNonNullElse($T.class.getPackage().getImplementationVersion(), \"unknown\")", + Objects.class, className) .build()); } @@ -190,6 +192,7 @@ private static List generateSimpleMetricFactory( .filter(tag -> tag.getValues().size() != 1) .map(tag -> ParameterSpec.builder( getTagClassName(metricName, tag), Custodian.sanitizeName(tag.getName())) + .addAnnotation(Safe.class) .build()) .collect(ImmutableList.toImmutableList())) .addJavadoc(Javadoc.render(definition.getDocs())); @@ -269,7 +272,10 @@ private static void generateMetricFactoryBuilder( outerBuilder.addType(TypeSpec.interfaceBuilder(stageName(metricName, tagName)) .addModifiers(visibility.apply()) .addMethod(methodBuilder - .addParameter(getTagClassName(metricName, tag), Custodian.sanitizeName(tagName)) + .addParameter(ParameterSpec.builder( + getTagClassName(metricName, tag), Custodian.sanitizeName(tagName)) + .addAnnotation(Safe.class) + .build()) .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT) .addAnnotation(CheckReturnValue.class) .returns(ClassName.bestGuess( @@ -336,7 +342,10 @@ private static void generateMetricFactoryBuilder( .addModifiers(Modifier.PUBLIC) .addAnnotation(Override.class) .returns(ClassName.bestGuess(Custodian.anyToUpperCamel(metricName) + "Builder")) - .addParameter(getTagClassName(metricName, tag), Custodian.sanitizeName(tag.getName())) + .addParameter(ParameterSpec.builder( + getTagClassName(metricName, tag), Custodian.sanitizeName(tag.getName())) + .addAnnotation(Safe.class) + .build()) .addStatement( "$1T.checkState(this.$2L == null, $3S)", Preconditions.class,