From 6d584e352e41d377ff9207319a0266b16a3dcf2d Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Thu, 3 Oct 2019 20:00:31 -0500 Subject: [PATCH 01/25] add New Relic Java Agent API dependency v5.+ --- implementations/micrometer-registry-new-relic/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/implementations/micrometer-registry-new-relic/build.gradle b/implementations/micrometer-registry-new-relic/build.gradle index 26d2c446ad..9dd5203509 100644 --- a/implementations/micrometer-registry-new-relic/build.gradle +++ b/implementations/micrometer-registry-new-relic/build.gradle @@ -1,6 +1,7 @@ dependencies { compile project(':micrometer-core') compile 'org.slf4j:slf4j-api:1.7.+' + compile 'com.newrelic.agent.java:newrelic-api:5.+' testCompile project(':micrometer-test') } \ No newline at end of file From 314433eb54626b974524cfcbcbfe547ee9283346 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Thu, 3 Oct 2019 22:39:49 -0500 Subject: [PATCH 02/25] Introduce ClientProvider interface - Migrate existing write/publish code to Http ClientProvider impl - Move meter output constants to interface for sharing --- .../newrelic/NewRelicClientProvider.java | 88 +++++ .../micrometer/newrelic/NewRelicConfig.java | 12 +- .../NewRelicHttpClientProviderImpl.java | 300 +++++++++++++++++ .../newrelic/NewRelicMeterRegistry.java | 313 +++--------------- 4 files changed, 440 insertions(+), 273 deletions(-) create mode 100644 implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java create mode 100644 implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java new file mode 100644 index 0000000000..8d90bdf543 --- /dev/null +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java @@ -0,0 +1,88 @@ +/** + * Copyright 2017 Pivotal Software, Inc. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micrometer.newrelic; + +import java.util.List; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.DistributionSummary; +import io.micrometer.core.instrument.FunctionCounter; +import io.micrometer.core.instrument.FunctionTimer; +import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.LongTaskTimer; +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.TimeGauge; +import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.config.NamingConvention; + +/** + * @author Neil Powell + */ +public interface NewRelicClientProvider { + //long task timer + static final String DURATION = "duration"; + static final String ACTIVE_TASKS = "activeTasks"; + //distribution summary & timer + static final String MAX = "max"; + static final String TOTAL = "total"; + static final String AVG = "avg"; + static final String COUNT = "count"; + //timer + static final String TOTAL_TIME = "totalTime"; + static final String TIME = "time"; + //gauge + static final String VALUE = "value"; + //counter + static final String THROUGHPUT = "throughput"; //TODO Why not "count"? ..confusing if just counting something + //timer + static final String TIME_UNIT = "timeUnit"; + //all + static final String METER_TYPE = "meterType"; + static final String METER_NAME = "meterName"; + + default String getEventType(Meter.Id id, NewRelicConfig config, NamingConvention namingConvention) { + String eventType = null; + if(config.meterNameEventTypeEnabled()) { + //meter/metric name event type + eventType = id.getConventionName(namingConvention); + } else { + //static eventType "category" + eventType = config.eventType(); + } + return eventType; + } + + void publish(MeterRegistry meterRegistry, List meters); + + Object writeFunctionTimer(FunctionTimer timer); + + Object writeTimer(Timer timer); + + Object writeSummary(DistributionSummary summary); + + Object writeLongTaskTimer(LongTaskTimer timer); + + Object writeTimeGauge(TimeGauge gauge); + + Object writeGauge(Gauge gauge); + + Object writeCounter(Counter counter); + + Object writeFunctionCounter(FunctionCounter counter); + + Object writeMeter(Meter meter); +} diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicConfig.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicConfig.java index f7989f3fa3..f37aa51eaf 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicConfig.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicConfig.java @@ -22,6 +22,7 @@ * Configuration for {@link NewRelicMeterRegistry}. * * @author Jon Schneider + * @author Neil Powell * @since 1.0.0 */ public interface NewRelicConfig extends StepRegistryConfig { @@ -53,17 +54,18 @@ default String eventType() { return v; } +// public interface Http extends NewRelicConfig { default String apiKey() { String v = get(prefix() + ".apiKey"); - if (v == null) - throw new MissingRequiredConfigurationException("apiKey must be set to report metrics to New Relic"); +// if (v == null) +// throw new MissingRequiredConfigurationException("apiKey must be set to report metrics to New Relic"); return v; } default String accountId() { String v = get(prefix() + ".accountId"); - if (v == null) - throw new MissingRequiredConfigurationException("accountId must be set to report metrics to New Relic"); +// if (v == null) +// throw new MissingRequiredConfigurationException("accountId must be set to report metrics to New Relic"); return v; } @@ -76,4 +78,6 @@ default String uri() { String v = get(prefix() + ".uri"); return (v == null) ? "https://insights-collector.newrelic.com" : v; } +// } + } diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java new file mode 100644 index 0000000000..3d92ec6c62 --- /dev/null +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java @@ -0,0 +1,300 @@ +/** + * Copyright 2017 Pivotal Software, Inc. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micrometer.newrelic; + +import static io.micrometer.core.instrument.util.StringEscapeUtils.escapeJson; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.DistributionSummary; +import io.micrometer.core.instrument.FunctionCounter; +import io.micrometer.core.instrument.FunctionTimer; +import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.LongTaskTimer; +import io.micrometer.core.instrument.Measurement; +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Tags; +import io.micrometer.core.instrument.TimeGauge; +import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; +import io.micrometer.core.instrument.config.NamingConvention; +import io.micrometer.core.instrument.util.DoubleFormat; +import io.micrometer.core.instrument.util.MeterPartition; +import io.micrometer.core.ipc.http.HttpSender; +import io.micrometer.core.ipc.http.HttpUrlConnectionSender; + +/** + * Publishes metrics to New Relic Insights REST API. + * + * @author Jon Schneider + * @author Johnny Lim + * @author Neil Powell + */ +public class NewRelicHttpClientProviderImpl implements NewRelicClientProvider { + + private final Logger logger = LoggerFactory.getLogger(NewRelicHttpClientProviderImpl.class); + + private final NewRelicConfig config; + private final HttpSender httpClient; + private final String insightsEndpoint; + private final NamingConvention namingConvention; + private final TimeUnit timeUnit; + + @SuppressWarnings("deprecation") + public NewRelicHttpClientProviderImpl(NewRelicConfig config) { + this(config, new HttpUrlConnectionSender(config.connectTimeout(), config.readTimeout()), new NewRelicNamingConvention(), TimeUnit.MILLISECONDS); + } + + public NewRelicHttpClientProviderImpl(NewRelicConfig config, HttpSender httpClient, NamingConvention namingConvention, TimeUnit timeUnit) { + + if (config.meterNameEventTypeEnabled() == false + && (config.eventType() == null || config.eventType().isEmpty())) { + throw new MissingRequiredConfigurationException("eventType must be set to report metrics to New Relic"); + } + if (config.accountId() == null || config.accountId().isEmpty()) { + throw new MissingRequiredConfigurationException("accountId must be set to report metrics to New Relic"); + } + if (config.apiKey() == null || config.apiKey().isEmpty()) { + throw new MissingRequiredConfigurationException("apiKey must be set to report metrics to New Relic"); + } + if (config.uri() == null || config.uri().isEmpty()) { + throw new MissingRequiredConfigurationException("uri must be set to report metrics to New Relic"); + } + + this.config = config; + this.httpClient = httpClient; + this.namingConvention = namingConvention; + this.timeUnit = timeUnit; + this.insightsEndpoint = config.uri() + "/v1/accounts/" + config.accountId() + "/events"; + } + + @Override + public void publish(MeterRegistry meterRegistry, List meters) { + // New Relic's Insights API limits us to 1000 events per call + // 1:1 mapping between Micrometer meters and New Relic events + for (List batch : MeterPartition.partition(meterRegistry, Math.min(config.batchSize(), 1000))) { + for (Meter meter : batch) { + sendEvents(meter.getId(), + meter.match( + this::writeGauge, + this::writeCounter, + this::writeTimer, + this::writeSummary, + this::writeLongTaskTimer, + this::writeTimeGauge, + this::writeFunctionCounter, + this::writeFunctionTimer, + this::writeMeter + ) + ); + } + } + } + + @Override + public Stream writeLongTaskTimer(LongTaskTimer ltt) { + return Stream.of( + event(ltt.getId(), + new Attribute(ACTIVE_TASKS, ltt.activeTasks()), + new Attribute(DURATION, ltt.duration(timeUnit)), + new Attribute(TIME_UNIT, timeUnit.name().toLowerCase()) + ) + ); + } + + @Override + public Stream writeFunctionCounter(FunctionCounter counter) { + double count = counter.count(); + if (Double.isFinite(count)) { + return Stream.of(event(counter.getId(), new Attribute(THROUGHPUT, count))); + } + return Stream.empty(); + } + + @Override + public Stream writeCounter(Counter counter) { + //TODO: Double.isFinite() check here like writeFunctionCounter ??? + return Stream.of(event(counter.getId(), new Attribute(THROUGHPUT, counter.count()))); + } + + @Override + public Stream writeGauge(Gauge gauge) { + Double value = gauge.value(); + if (Double.isFinite(value)) { + return Stream.of(event(gauge.getId(), new Attribute(VALUE, value))); + } + return Stream.empty(); + } + + @Override + public Stream writeTimeGauge(TimeGauge gauge) { + Double value = gauge.value(timeUnit); + if (Double.isFinite(value)) { + return Stream.of( + event(gauge.getId(), + new Attribute(VALUE, value), + new Attribute(TIME_UNIT, timeUnit.name().toLowerCase()) + ) + ); + } + return Stream.empty(); + } + + @Override + public Stream writeSummary(DistributionSummary summary) { + return Stream.of( + event(summary.getId(), + new Attribute(COUNT, summary.count()), + new Attribute(AVG, summary.mean()), + new Attribute(TOTAL, summary.totalAmount()), + new Attribute(MAX, summary.max()) + ) + ); + } + + @Override + public Stream writeTimer(Timer timer) { + return Stream.of( + event(timer.getId(), + new Attribute(COUNT, timer.count()), + new Attribute(AVG, timer.mean(timeUnit)), + new Attribute(TOTAL_TIME, timer.totalTime(timeUnit)), + new Attribute(MAX, timer.max(timeUnit)), + new Attribute(TIME_UNIT, timeUnit.name().toLowerCase()) + ) + ); + } + + @Override + public Stream writeFunctionTimer(FunctionTimer timer) { + return Stream.of( + event(timer.getId(), + new Attribute(COUNT, timer.count()), + new Attribute(AVG, timer.mean(timeUnit)), + new Attribute(TOTAL_TIME, timer.totalTime(timeUnit)), + new Attribute(TIME_UNIT, timeUnit.name().toLowerCase()) + ) + ); + } + + @Override + public Stream writeMeter(Meter meter) { + // Snapshot values should be used throughout this method as there are chances for values to be changed in-between. + Map attributes = new HashMap<>(); + for (Measurement measurement : meter.measure()) { + double value = measurement.getValue(); + if (!Double.isFinite(value)) { + continue; + } + String name = measurement.getStatistic().getTagValueRepresentation(); + attributes.put(name, new Attribute(name, value)); + } + if (attributes.isEmpty()) { + return Stream.empty(); + } + return Stream.of(event(meter.getId(), attributes.values().toArray(new Attribute[0]))); + } + + private String event(Meter.Id id, Attribute... attributes) { + if (!config.meterNameEventTypeEnabled()) { + //Include these if NOT generating an event per Meter/metric name + int size = attributes.length; + Attribute[] newAttrs = Arrays.copyOf(attributes, size + 2); + + String name = id.getConventionName(namingConvention); + newAttrs[size] = new Attribute(METER_NAME, name); + newAttrs[size + 1] = new Attribute(METER_TYPE, id.getType().toString()); + + return event(id, Tags.empty(), newAttrs); + } + + return event(id, Tags.empty(), attributes); + } + + private String event(Meter.Id id, Iterable extraTags, Attribute... attributes) { + StringBuilder tagsJson = new StringBuilder(); + + for (Tag tag : id.getConventionTags(namingConvention)) { + tagsJson.append(",\"").append(escapeJson(tag.getKey())).append("\":\"").append(escapeJson(tag.getValue())).append("\""); + } + + for (Tag tag : extraTags) { + tagsJson.append(",\"").append(escapeJson(namingConvention.tagKey(tag.getKey()))) + .append("\":\"").append(escapeJson(namingConvention.tagValue(tag.getValue()))).append("\""); + } + + String eventType = getEventType(id, config, namingConvention); + + return Arrays.stream(attributes) + .map(attr -> + (attr.getValue() instanceof Number) + ? ",\"" + attr.getName() + "\":" + DoubleFormat.wholeOrDecimal(((Number)attr.getValue()).doubleValue()) + : ",\"" + attr.getName() + "\":\"" + namingConvention.tagValue(attr.getValue().toString()) + "\"" + ) + .collect(Collectors.joining("", "{\"eventType\":\"" + escapeJson(eventType) + "\"", tagsJson + "}")); + } + + void sendEvents(Meter.Id id, Object attributesObj) { + + if (attributesObj instanceof Stream) { + @SuppressWarnings("unchecked") + Stream events = (Stream)attributesObj; + try { + AtomicInteger totalEvents = new AtomicInteger(); + + httpClient.post(insightsEndpoint) + .withHeader("X-Insert-Key", config.apiKey()) + .withJsonContent(events.peek(ev -> totalEvents.incrementAndGet()).collect(Collectors.joining(",", "[", "]"))) + .send() + .onSuccess(response -> logger.debug("successfully sent {} metrics to New Relic.", totalEvents)) + .onError(response -> logger.error("failed to send metrics to new relic: http {} {}", response.code(), response.body())); + } catch (Throwable e) { + logger.warn("failed to send metrics to new relic", e); + } + } + } + + private class Attribute { + private final String name; + private final Object value; + + private Attribute(String name, Object value) { + this.name = name; + this.value = value; + } + + public String getName() { + return name; + } + + public Object getValue() { + return value; + } + } +} diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java index df34989b1b..8de6b0aae1 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java @@ -15,294 +15,92 @@ */ package io.micrometer.newrelic; -import io.micrometer.core.instrument.*; -import io.micrometer.core.instrument.Timer; -import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; -import io.micrometer.core.instrument.config.NamingConvention; -import io.micrometer.core.instrument.step.StepMeterRegistry; -import io.micrometer.core.instrument.util.DoubleFormat; -import io.micrometer.core.instrument.util.MeterPartition; -import io.micrometer.core.instrument.util.NamedThreadFactory; -import io.micrometer.core.instrument.util.TimeUtils; -import io.micrometer.core.ipc.http.HttpSender; -import io.micrometer.core.ipc.http.HttpUrlConnectionSender; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.util.*; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.stream.Collectors; -import java.util.stream.Stream; -import static io.micrometer.core.instrument.util.StringEscapeUtils.escapeJson; +import io.micrometer.core.instrument.Clock; +import io.micrometer.core.instrument.config.NamingConvention; +import io.micrometer.core.instrument.step.StepMeterRegistry; +import io.micrometer.core.instrument.util.NamedThreadFactory; /** - * Publishes metrics to New Relic Insights. + * Publishes metrics to New Relic Insights based on provider selected. * * @author Jon Schneider * @author Johnny Lim - * @since 1.0.0 + * @author Neil Powell */ public class NewRelicMeterRegistry extends StepMeterRegistry { + private static final ThreadFactory DEFAULT_THREAD_FACTORY = new NamedThreadFactory("new-relic-metrics-publisher"); - private final NewRelicConfig config; - private final HttpSender httpClient; - private final Logger logger = LoggerFactory.getLogger(NewRelicMeterRegistry.class); + private NewRelicClientProvider clientProvider; /** * @param config Configuration options for the registry that are describable as properties. + * @param clientProvider Provider of the HTTP or Agent-based client that publishes metrics to New Relic * @param clock The clock to use for timings. */ - @SuppressWarnings("deprecation") - public NewRelicMeterRegistry(NewRelicConfig config, Clock clock) { - this(config, clock, DEFAULT_THREAD_FACTORY, - new HttpUrlConnectionSender(config.connectTimeout(), config.readTimeout())); + public NewRelicMeterRegistry(NewRelicConfig config, NewRelicClientProvider clientProvider, Clock clock) { + this(config,clientProvider, new NewRelicNamingConvention(), clock, DEFAULT_THREAD_FACTORY); } /** - * @param config Configuration options for the registry that are describable as properties. - * @param clock The clock to use for timings. + * @param config Configuration options for the registry that are describable as properties. + * @param clientProvider Provider of the HTTP or Agent-based client that publishes metrics to New Relic + * @param namingConvention Naming convention to apply before metric publishing + * @param clock The clock to use for timings. * @param threadFactory The thread factory to use to create the publishing thread. - * @deprecated Use {@link #builder(NewRelicConfig)} instead. + * @deprecated Use {@link #builder(NewNewRelicConfig)} instead. */ @Deprecated - public NewRelicMeterRegistry(NewRelicConfig config, Clock clock, ThreadFactory threadFactory) { - this(config, clock, threadFactory, new HttpUrlConnectionSender(config.connectTimeout(), config.readTimeout())); - } - - // VisibleForTesting - NewRelicMeterRegistry(NewRelicConfig config, Clock clock, ThreadFactory threadFactory, HttpSender httpClient) { + public NewRelicMeterRegistry(NewRelicConfig config, NewRelicClientProvider clientProvider, + NamingConvention convention, Clock clock, ThreadFactory threadFactory) { super(config, clock); - if (config.meterNameEventTypeEnabled() == false - && (config.eventType() == null || config.eventType().isEmpty())) { - throw new MissingRequiredConfigurationException("eventType must be set to report metrics to New Relic"); - } - if (config.accountId() == null || config.accountId().isEmpty()) { - throw new MissingRequiredConfigurationException("accountId must be set to report metrics to New Relic"); - } - if (config.apiKey() == null || config.apiKey().isEmpty()) { - throw new MissingRequiredConfigurationException("apiKey must be set to report metrics to New Relic"); - } - if (config.uri() == null || config.uri().isEmpty()) { - throw new MissingRequiredConfigurationException("uri must be set to report metrics to New Relic"); - } + this.clientProvider = clientProvider; - this.config = config; - this.httpClient = httpClient; - - config().namingConvention(new NewRelicNamingConvention()); + config().namingConvention(convention); start(threadFactory); } - - public static Builder builder(NewRelicConfig config) { - return new Builder(config); - } - - @Override - public void start(ThreadFactory threadFactory) { - if (config.enabled()) { - logger.info("publishing metrics to new relic every " + TimeUtils.format(config.step())); - } - super.start(threadFactory); - } - + @Override protected void publish() { - String insightsEndpoint = config.uri() + "/v1/accounts/" + config.accountId() + "/events"; - - // New Relic's Insights API limits us to 1000 events per call - // 1:1 mapping between Micrometer meters and New Relic events - for (List batch : MeterPartition.partition(this, Math.min(config.batchSize(), 1000))) { - sendEvents(insightsEndpoint, batch.stream().flatMap(meter -> meter.match( - this::writeGauge, - this::writeCounter, - this::writeTimer, - this::writeSummary, - this::writeLongTaskTimer, - this::writeTimeGauge, - this::writeFunctionCounter, - this::writeFunctionTimer, - this::writeMeter))); - } - } - - private Stream writeLongTaskTimer(LongTaskTimer ltt) { - return Stream.of( - event(ltt.getId(), - new Attribute("activeTasks", ltt.activeTasks()), - new Attribute("duration", ltt.duration(getBaseTimeUnit())), - new Attribute("timeUnit", getBaseTimeUnit().name().toLowerCase())) - ); - } - - // VisibleForTesting - Stream writeFunctionCounter(FunctionCounter counter) { - double count = counter.count(); - if (Double.isFinite(count)) { - return Stream.of(event(counter.getId(), new Attribute("throughput", count))); - } - return Stream.empty(); - } - - private Stream writeCounter(Counter counter) { - //TODO: Double.isFinite() check here like writeFunctionCounter ??? - return Stream.of(event(counter.getId(), new Attribute("throughput", counter.count()))); - } - - // VisibleForTesting - Stream writeGauge(Gauge gauge) { - Double value = gauge.value(); - if (Double.isFinite(value)) { - return Stream.of(event(gauge.getId(), new Attribute("value", value))); - } - return Stream.empty(); - } - - // VisibleForTesting - Stream writeTimeGauge(TimeGauge gauge) { - Double value = gauge.value(getBaseTimeUnit()); - if (Double.isFinite(value)) { - return Stream.of( - event(gauge.getId(), - new Attribute("value", value), - new Attribute("timeUnit", getBaseTimeUnit().name().toLowerCase()))); - } - return Stream.empty(); - } - - private Stream writeSummary(DistributionSummary summary) { - return Stream.of( - event(summary.getId(), - new Attribute("count", summary.count()), - new Attribute("avg", summary.mean()), - new Attribute("total", summary.totalAmount()), - new Attribute("max", summary.max()) - ) - ); - } - - private Stream writeTimer(Timer timer) { - return Stream.of(event(timer.getId(), - new Attribute("count", timer.count()), - new Attribute("avg", timer.mean(getBaseTimeUnit())), - new Attribute("totalTime", timer.totalTime(getBaseTimeUnit())), - new Attribute("max", timer.max(getBaseTimeUnit())), - new Attribute("timeUnit", getBaseTimeUnit().name().toLowerCase()) - )); - } - - private Stream writeFunctionTimer(FunctionTimer timer) { - return Stream.of( - event(timer.getId(), - new Attribute("count", timer.count()), - new Attribute("avg", timer.mean(getBaseTimeUnit())), - new Attribute("totalTime", timer.totalTime(getBaseTimeUnit())), - new Attribute("timeUnit", getBaseTimeUnit().name().toLowerCase()) - ) - ); - } - - // VisibleForTesting - Stream writeMeter(Meter meter) { - // Snapshot values should be used throughout this method as there are chances for values to be changed in-between. - Map attributes = new HashMap<>(); - for (Measurement measurement : meter.measure()) { - double value = measurement.getValue(); - if (!Double.isFinite(value)) { - continue; - } - String name = measurement.getStatistic().getTagValueRepresentation(); - attributes.put(name, new Attribute(name, value)); - } - if (attributes.isEmpty()) { - return Stream.empty(); - } - return Stream.of(event(meter.getId(), attributes.values().toArray(new Attribute[0]))); - } - - private String event(Meter.Id id, Attribute... attributes) { - if (!config.meterNameEventTypeEnabled()) { - // Include contextual attributes when publishing all metrics under a single categorical eventType, - // NOT when publishing an eventType per Meter/metric name - int size = attributes.length; - Attribute[] newAttrs = Arrays.copyOf(attributes, size + 2); - - String name = id.getConventionName(config().namingConvention()); - newAttrs[size] = new Attribute("metricName", name); - newAttrs[size + 1] = new Attribute("metricType", id.getType().toString()); - - return event(id, Tags.empty(), newAttrs); - } - return event(id, Tags.empty(), attributes); - } - - private String event(Meter.Id id, Iterable extraTags, Attribute... attributes) { - StringBuilder tagsJson = new StringBuilder(); - - for (Tag tag : getConventionTags(id)) { - tagsJson.append(",\"").append(escapeJson(tag.getKey())).append("\":\"").append(escapeJson(tag.getValue())).append("\""); - } - - NamingConvention convention = config().namingConvention(); - for (Tag tag : extraTags) { - tagsJson.append(",\"").append(escapeJson(convention.tagKey(tag.getKey()))) - .append("\":\"").append(escapeJson(convention.tagValue(tag.getValue()))).append("\""); - } - - String eventType = getEventType(id); - - return Arrays.stream(attributes) - .map(attr -> - (attr.getValue() instanceof Number) - ? ",\"" + attr.getName() + "\":" + DoubleFormat.wholeOrDecimal(((Number)attr.getValue()).doubleValue()) - : ",\"" + attr.getName() + "\":\"" + convention.tagValue(attr.getValue().toString()) + "\"" - ) - .collect(Collectors.joining("", "{\"eventType\":\"" + escapeJson(eventType) + "\"", tagsJson + "}")); - } - - private String getEventType(Meter.Id id) { - if (config.meterNameEventTypeEnabled()) { - return id.getConventionName(config().namingConvention()); - } else { - return config.eventType(); - } - } - - private void sendEvents(String insightsEndpoint, Stream events) { - try { - AtomicInteger totalEvents = new AtomicInteger(); - - httpClient.post(insightsEndpoint) - .withHeader("X-Insert-Key", config.apiKey()) - .withJsonContent(events.peek(ev -> totalEvents.incrementAndGet()).collect(Collectors.joining(",", "[", "]"))) - .send() - .onSuccess(response -> logger.debug("successfully sent {} metrics to New Relic.", totalEvents)) - .onError(response -> logger.error("failed to send metrics to new relic: http {} {}", response.code(), response.body())); - } catch (Throwable e) { - logger.warn("failed to send metrics to new relic", e); - } + clientProvider.publish(this, getMeters()); } @Override protected TimeUnit getBaseTimeUnit() { - return TimeUnit.SECONDS; + return TimeUnit.MILLISECONDS; } public static class Builder { private final NewRelicConfig config; + private NewRelicClientProvider clientProvider; + private NamingConvention convention = new NewRelicNamingConvention(); private Clock clock = Clock.SYSTEM; private ThreadFactory threadFactory = DEFAULT_THREAD_FACTORY; - private HttpSender httpClient; - @SuppressWarnings("deprecation") Builder(NewRelicConfig config) { this.config = config; - this.httpClient = new HttpUrlConnectionSender(config.connectTimeout(), config.readTimeout()); + } + +// public Builder agentClientProvider() { +// return clientProvider(new NewRelicAgentClientProviderImpl(config)); +// } + + public Builder httpClientProvider() { + return clientProvider(new NewRelicHttpClientProviderImpl(config)); + } + + public Builder clientProvider(NewRelicClientProvider clientProvider) { + this.clientProvider = clientProvider; + return this; + } + + public Builder namingConvention(NamingConvention convention) { + this.convention = convention; + return this; } public Builder clock(Clock clock) { @@ -315,31 +113,8 @@ public Builder threadFactory(ThreadFactory threadFactory) { return this; } - public Builder httpClient(HttpSender httpClient) { - this.httpClient = httpClient; - return this; - } - public NewRelicMeterRegistry build() { - return new NewRelicMeterRegistry(config, clock, threadFactory, httpClient); - } - } - - private class Attribute { - private final String name; - private final Object value; - - private Attribute(String name, Object value) { - this.name = name; - this.value = value; - } - - public String getName() { - return name; - } - - public Object getValue() { - return value; + return new NewRelicMeterRegistry(config, clientProvider, convention, clock, threadFactory); } } } From a479eb0f3e620391a8a158fd93a74f79545cf551 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Sun, 6 Oct 2019 22:17:11 -0500 Subject: [PATCH 03/25] Add Java Agent ClientProvider impl and update tests --- .../NewRelicAgentClientProviderImpl.java | 256 +++++++ .../newrelic/NewRelicClientProvider.java | 4 +- .../NewRelicHttpClientProviderImpl.java | 4 +- .../newrelic/NewRelicMeterRegistry.java | 38 +- .../newrelic/NewRelicMeterRegistryTest.java | 679 +++++++++++++++--- 5 files changed, 853 insertions(+), 128 deletions(-) create mode 100644 implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java new file mode 100644 index 0000000000..431fd086e4 --- /dev/null +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java @@ -0,0 +1,256 @@ +/** + * Copyright 2017 Pivotal Software, Inc. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micrometer.newrelic; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.newrelic.api.agent.Agent; +import com.newrelic.api.agent.NewRelic; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.DistributionSummary; +import io.micrometer.core.instrument.FunctionCounter; +import io.micrometer.core.instrument.FunctionTimer; +import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.LongTaskTimer; +import io.micrometer.core.instrument.Measurement; +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.TimeGauge; +import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; +import io.micrometer.core.instrument.config.NamingConvention; +import io.micrometer.core.instrument.util.StringUtils; + +/** + * Publishes metrics to New Relic Insights via Java Agent API. + * + * @author Neil Powell + */ +public class NewRelicAgentClientProviderImpl implements NewRelicClientProvider { + + private final Logger logger = LoggerFactory.getLogger(NewRelicAgentClientProviderImpl.class); + + private final Agent newRelicAgent; + private final NewRelicConfig config; + private final NamingConvention namingConvention; + private final TimeUnit timeUnit; + + public NewRelicAgentClientProviderImpl(NewRelicConfig config) { + this(config, NewRelic.getAgent(), new NewRelicNamingConvention(), TimeUnit.MILLISECONDS); + } + + public NewRelicAgentClientProviderImpl(NewRelicConfig config, Agent newRelicAgent, NamingConvention namingConvention, TimeUnit timeUnit) { + + if (config.meterNameEventTypeEnabled() == false + && StringUtils.isEmpty(config.eventType())) { + throw new MissingRequiredConfigurationException("eventType must be set to report metrics to New Relic"); + } + + this.newRelicAgent = newRelicAgent; + this.config = config; + this.namingConvention = namingConvention; + this.timeUnit = timeUnit; + } + + @Override + public Map writeLongTaskTimer(LongTaskTimer timer) { + Map attributes = new HashMap(); + addAttribute(ACTIVE_TASKS, timer.activeTasks(), attributes); + addAttribute(DURATION, timer.duration(timeUnit), attributes); + addAttribute(TIME_UNIT, timeUnit.name().toLowerCase(), attributes); + //process meter's name, type and tags + addMeterAsAttributes(timer.getId(), attributes); + return attributes; + } + + @Override + public Map writeFunctionCounter(FunctionCounter counter) { + return writeCounterValues(counter.getId(), counter.count()); + } + + @Override + public Map writeCounter(Counter counter) { + return writeCounterValues(counter.getId(), counter.count()); + } + + Map writeCounterValues(Meter.Id id, double count) { + Map attributes = new HashMap(); + if (Double.isFinite(count)) { + addAttribute(THROUGHPUT, count, attributes); //TODO Why not named Count? ..confusing output! + //process meter's name, type and tags + addMeterAsAttributes(id, attributes); + } + return attributes; + } + + @Override + public Map writeGauge(Gauge gauge) { + Map attributes = new HashMap(); + double value = gauge.value(); + if (Double.isFinite(value)) { + addAttribute(VALUE, value, attributes); + //process meter's name, type and tags + addMeterAsAttributes(gauge.getId(), attributes); + } + return attributes; + } + + @Override + public Map writeTimeGauge(TimeGauge gauge) { + Map attributes = new HashMap(); + double value = gauge.value(timeUnit); + if (Double.isFinite(value)) { + addAttribute(VALUE, value, attributes); + addAttribute(TIME_UNIT, gauge.baseTimeUnit().name().toLowerCase(), attributes); + //process meter's name, type and tags + addMeterAsAttributes(gauge.getId(), attributes); + } + return attributes; + } + + @Override + public Map writeSummary(DistributionSummary summary) { + Map attributes = new HashMap(); + addAttribute(COUNT, summary.count(), attributes); + addAttribute(AVG, summary.mean(), attributes); + addAttribute(TOTAL, summary.totalAmount(), attributes); + addAttribute(MAX, summary.max(), attributes); + //process meter's name, type and tags + addMeterAsAttributes(summary.getId(), attributes); + return attributes; + } + + @Override + public Map writeTimer(Timer timer) { + Map attributes = new HashMap(); + addAttribute(COUNT, (new Double(timer.count())).longValue(), attributes); + addAttribute(AVG, timer.mean(timeUnit), attributes); + addAttribute(TOTAL_TIME, timer.totalTime(timeUnit), attributes); + addAttribute(MAX, timer.max(timeUnit), attributes); + addAttribute(TIME_UNIT, timeUnit.name().toLowerCase(), attributes); + //process meter's name, type and tags + addMeterAsAttributes(timer.getId(), attributes); + return attributes; + } + + @Override + public Map writeFunctionTimer(FunctionTimer timer) { + Map attributes = new HashMap(); + addAttribute(COUNT, (new Double(timer.count())).longValue(), attributes); + addAttribute(AVG, timer.mean(timeUnit), attributes); + addAttribute(TOTAL_TIME, timer.totalTime(timeUnit), attributes); + addAttribute(TIME_UNIT, timeUnit.name().toLowerCase(), attributes); + //process meter's name, type and tags + addMeterAsAttributes(timer.getId(), attributes); + return attributes; + } + + @Override + public Map writeMeter(Meter meter) { + Map attributes = new HashMap(); + for (Measurement measurement : meter.measure()) { + double value = measurement.getValue(); + if (!Double.isFinite(value)) { + continue; + } + addAttribute(measurement.getStatistic().getTagValueRepresentation(), value, attributes); + } + if (attributes.isEmpty()) { + return attributes; + } + //process meter's name, type and tags + addMeterAsAttributes(meter.getId(), attributes); + return attributes; + } + + void addMeterAsAttributes(Meter.Id id, Map attributes) { + if (config.meterNameEventTypeEnabled() == false) { + //Include these if NOT generating an event per Meter/metric name + String name = id.getConventionName(namingConvention); // same as getConventionName( meter.getId() ) + attributes.put(METRIC_NAME, name); + attributes.put(METRIC_TYPE, id.getType().toString()); + } + //process meter tags + for (Tag tag : id.getConventionTags(namingConvention)) { + attributes.put(tag.getKey(), tag.getValue()); + } + } + + void addAttribute(String key, Number value, Map attributes) { + //process other tags + + //Replicate DoubleFormat.wholeOrDecimal(value.doubleValue()) formatting behavior + if (Math.floor(value.doubleValue()) == value.doubleValue()) { + //whole number - don't include decimal + attributes.put(namingConvention.tagKey(key), value.intValue()); + } else { + //include decimal + attributes.put(namingConvention.tagKey(key), value.doubleValue()); + } + } + + void addAttribute(String key, String value, Map attributes) { + //process other tags + attributes.put(namingConvention.tagKey(key), namingConvention.tagValue(value)); + } + + @Override + public void publish(MeterRegistry meterRegistry, List meters) { + // New Relic's Java Agent Insights API is backed by a reservoir/buffer + // and handles the actual publishing of events to New Relic. + // 1:1 mapping between Micrometer meters and New Relic events + for (Meter meter : meters) { + sendEvents( + meter.getId(), + meter.match( + this::writeGauge, + this::writeCounter, + this::writeTimer, + this::writeSummary, + this::writeLongTaskTimer, + this::writeTimeGauge, + this::writeFunctionCounter, + this::writeFunctionTimer, + this::writeMeter) + ); + } + } + + void sendEvents(Meter.Id id, Object attributesObj) { + if (attributesObj instanceof Map) { + @SuppressWarnings("unchecked") + Map attributes = (Map)attributesObj; + + //Delegate to New Relic Java Agent + if(attributes != null && attributes.isEmpty() == false) { + String eventType = getEventType(id, config, namingConvention); + try { + newRelicAgent.getInsights().recordCustomEvent(eventType, attributes); + } catch (Throwable e) { + logger.warn("failed to send metrics to new relic", e); + } + } + } + } +} diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java index 8d90bdf543..52e4bf2d4b 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java @@ -51,8 +51,8 @@ public interface NewRelicClientProvider { //timer static final String TIME_UNIT = "timeUnit"; //all - static final String METER_TYPE = "meterType"; - static final String METER_NAME = "meterName"; + static final String METRIC_TYPE = "metricType"; + static final String METRIC_NAME = "metricName"; default String getEventType(Meter.Id id, NewRelicConfig config, NamingConvention namingConvention) { String eventType = null; diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java index 3d92ec6c62..4a7a185971 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java @@ -228,8 +228,8 @@ private String event(Meter.Id id, Attribute... attributes) { Attribute[] newAttrs = Arrays.copyOf(attributes, size + 2); String name = id.getConventionName(namingConvention); - newAttrs[size] = new Attribute(METER_NAME, name); - newAttrs[size + 1] = new Attribute(METER_TYPE, id.getType().toString()); + newAttrs[size] = new Attribute(METRIC_NAME, name); + newAttrs[size + 1] = new Attribute(METRIC_TYPE, id.getType().toString()); return event(id, Tags.empty(), newAttrs); } diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java index 8de6b0aae1..3863891cdd 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java @@ -19,6 +19,7 @@ import java.util.concurrent.TimeUnit; import io.micrometer.core.instrument.Clock; +import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; import io.micrometer.core.instrument.config.NamingConvention; import io.micrometer.core.instrument.step.StepMeterRegistry; import io.micrometer.core.instrument.util.NamedThreadFactory; @@ -35,13 +36,21 @@ public class NewRelicMeterRegistry extends StepMeterRegistry { private static final ThreadFactory DEFAULT_THREAD_FACTORY = new NamedThreadFactory("new-relic-metrics-publisher"); private NewRelicClientProvider clientProvider; + /** + * @param config Configuration options for the registry that are describable as properties. + * @param clock The clock to use for timings. + */ + public NewRelicMeterRegistry(NewRelicConfig config, Clock clock) { + this(config, new NewRelicHttpClientProviderImpl(config), clock); + } + /** * @param config Configuration options for the registry that are describable as properties. * @param clientProvider Provider of the HTTP or Agent-based client that publishes metrics to New Relic * @param clock The clock to use for timings. */ public NewRelicMeterRegistry(NewRelicConfig config, NewRelicClientProvider clientProvider, Clock clock) { - this(config,clientProvider, new NewRelicNamingConvention(), clock, DEFAULT_THREAD_FACTORY); + this(config, clientProvider, new NewRelicNamingConvention(), clock, DEFAULT_THREAD_FACTORY); } /** @@ -50,16 +59,26 @@ public NewRelicMeterRegistry(NewRelicConfig config, NewRelicClientProvider clien * @param namingConvention Naming convention to apply before metric publishing * @param clock The clock to use for timings. * @param threadFactory The thread factory to use to create the publishing thread. - * @deprecated Use {@link #builder(NewNewRelicConfig)} instead. + * @deprecated Use {@link #builder(NewRelicConfig)} instead. */ - @Deprecated - public NewRelicMeterRegistry(NewRelicConfig config, NewRelicClientProvider clientProvider, - NamingConvention convention, Clock clock, ThreadFactory threadFactory) { + @Deprecated // VisibleForTesting + NewRelicMeterRegistry(NewRelicConfig config, NewRelicClientProvider clientProvider, + NamingConvention namingConvention, Clock clock, ThreadFactory threadFactory) { super(config, clock); + if (clientProvider == null) { + throw new MissingRequiredConfigurationException("clientProvider required to report metrics to New Relic"); + } + if (namingConvention == null) { + throw new MissingRequiredConfigurationException("namingConvention must be set to report metrics to New Relic"); + } + if (threadFactory == null) { + throw new MissingRequiredConfigurationException("threadFactory must be set to report metrics to New Relic"); + } + this.clientProvider = clientProvider; - config().namingConvention(convention); + config().namingConvention(namingConvention); start(threadFactory); } @@ -83,11 +102,12 @@ public static class Builder { Builder(NewRelicConfig config) { this.config = config; + httpClientProvider(); } -// public Builder agentClientProvider() { -// return clientProvider(new NewRelicAgentClientProviderImpl(config)); -// } + public Builder agentClientProvider() { + return clientProvider(new NewRelicAgentClientProviderImpl(config)); + } public Builder httpClientProvider() { return clientProvider(new NewRelicHttpClientProviderImpl(config)); diff --git a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java index 1cbfeea755..1b35b31d65 100644 --- a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java +++ b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java @@ -20,39 +20,57 @@ import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import com.newrelic.api.agent.Agent; +import com.newrelic.api.agent.Config; +import com.newrelic.api.agent.Insights; +import com.newrelic.api.agent.Logger; +import com.newrelic.api.agent.MetricAggregator; +import com.newrelic.api.agent.TraceMetadata; +import com.newrelic.api.agent.TracedMethod; +import com.newrelic.api.agent.Transaction; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.DistributionSummary; import io.micrometer.core.instrument.FunctionCounter; +import io.micrometer.core.instrument.FunctionTimer; import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.LongTaskTimer; import io.micrometer.core.instrument.Measurement; import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.MockClock; import io.micrometer.core.instrument.Statistic; import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.TimeGauge; +import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; import io.micrometer.core.instrument.util.NamedThreadFactory; import io.micrometer.core.ipc.http.HttpSender; +import io.micrometer.newrelic.NewRelicMeterRegistryTest.MockNewRelicAgent.MockNewRelicInsights; /** * Tests for {@link NewRelicMeterRegistry}. * * @author Johnny Lim + * @author Neil Powell */ class NewRelicMeterRegistryTest { - private final NewRelicConfig config = new NewRelicConfig() { - + private final NewRelicConfig agentConfig = new NewRelicConfig() { @Override - public boolean meterNameEventTypeEnabled() { - //Default is false. Publish all metrics under a single eventType - return NewRelicConfig.super.meterNameEventTypeEnabled(); + public String get(String key) { + return null; } - + }; + + private final NewRelicConfig httpConfig = new NewRelicConfig() { @Override public String get(String key) { return null; @@ -62,22 +80,20 @@ public String get(String key) { public String accountId() { return "accountId"; } - + @Override public String apiKey() { return "apiKey"; } - }; - + private final NewRelicConfig meterNameEventTypeEnabledConfig = new NewRelicConfig() { - + @Override public boolean meterNameEventTypeEnabled() { - //Previous behavior for backward compatibility return true; } - + @Override public String get(String key) { return null; @@ -87,140 +103,191 @@ public String get(String key) { public String accountId() { return "accountId"; } - + @Override public String apiKey() { return "apiKey"; } - }; private final MockClock clock = new MockClock(); - private final NewRelicMeterRegistry meterNameEventTypeEnabledRegistry = new NewRelicMeterRegistry(meterNameEventTypeEnabledConfig, clock); - private final NewRelicMeterRegistry registry = new NewRelicMeterRegistry(config, clock); + private final NewRelicMeterRegistry registry = new NewRelicMeterRegistry(httpConfig, new MockClientProvider(), clock); + + NewRelicAgentClientProviderImpl getAgentClientProvider(NewRelicConfig config) { + return new NewRelicAgentClientProviderImpl(config); + } + NewRelicHttpClientProviderImpl getHttpClientProvider(NewRelicConfig config) { + return new NewRelicHttpClientProviderImpl(config); + } @Test void writeGauge() { - meterNameEventTypeEnabledRegistry.gauge("my.gauge", 1d); - Gauge gauge = meterNameEventTypeEnabledRegistry.find("my.gauge").gauge(); - Stream streamResult = meterNameEventTypeEnabledRegistry.writeGauge(gauge); - assertThat(streamResult).contains("{\"eventType\":\"myGauge\",\"value\":1}"); - registry.gauge("my.gauge", 1d); - gauge = registry.find("my.gauge").gauge(); - streamResult = registry.writeGauge(gauge); + Gauge gauge = registry.find("my.gauge").gauge(); + //test Http clientProvider + Stream streamResult = getHttpClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge); + assertThat(streamResult).contains("{\"eventType\":\"myGauge\",\"value\":1}"); + + streamResult = getHttpClientProvider(httpConfig).writeGauge(gauge); assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\"}"); + + //test Agent clientProvider + Map result = getAgentClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge); + assertThat(result).hasSize(1); + assertThat(result).containsEntry("value", 1); + + result = getAgentClientProvider(agentConfig).writeGauge(gauge); + assertThat(result).hasSize(3); + assertThat(result).containsEntry("metricName", "myGauge"); + assertThat(result).containsEntry("metricType", "GAUGE"); + assertThat(result).containsEntry("value", 1); } @Test void writeGaugeShouldDropNanValue() { - meterNameEventTypeEnabledRegistry.gauge("my.gauge", Double.NaN); - Gauge gauge = meterNameEventTypeEnabledRegistry.find("my.gauge").gauge(); - assertThat(meterNameEventTypeEnabledRegistry.writeGauge(gauge)).isEmpty(); - registry.gauge("my.gauge", Double.NaN); - gauge = registry.find("my.gauge").gauge(); - assertThat(registry.writeGauge(gauge)).isEmpty(); + Gauge gauge = registry.find("my.gauge").gauge(); + //test Http clientProvider + assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); + assertThat(getHttpClientProvider(httpConfig).writeGauge(gauge)).isEmpty(); + + //test Agent clientProvider + assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); + assertThat(getAgentClientProvider(agentConfig).writeGauge(gauge)).isEmpty(); } @Test void writeGaugeShouldDropInfiniteValues() { - meterNameEventTypeEnabledRegistry.gauge("my.gauge", Double.POSITIVE_INFINITY); - Gauge gauge = meterNameEventTypeEnabledRegistry.find("my.gauge").gauge(); - assertThat(meterNameEventTypeEnabledRegistry.writeGauge(gauge)).isEmpty(); - - meterNameEventTypeEnabledRegistry.gauge("my.gauge", Double.NEGATIVE_INFINITY); - gauge = meterNameEventTypeEnabledRegistry.find("my.gauge").gauge(); - assertThat(meterNameEventTypeEnabledRegistry.writeGauge(gauge)).isEmpty(); - registry.gauge("my.gauge", Double.POSITIVE_INFINITY); - gauge = registry.find("my.gauge").gauge(); - assertThat(registry.writeGauge(gauge)).isEmpty(); + Gauge gauge = registry.find("my.gauge").gauge(); + //test Http clientProvider + assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); + assertThat(getHttpClientProvider(httpConfig).writeGauge(gauge)).isEmpty(); + //test Agent clientProvider + assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); + assertThat(getAgentClientProvider(agentConfig).writeGauge(gauge)).isEmpty(); + registry.gauge("my.gauge", Double.NEGATIVE_INFINITY); gauge = registry.find("my.gauge").gauge(); - assertThat(registry.writeGauge(gauge)).isEmpty(); + //test Http clientProvider + assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); + assertThat(getHttpClientProvider(httpConfig).writeGauge(gauge)).isEmpty(); + + //test Agent clientProvider + assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); + assertThat(getAgentClientProvider(agentConfig).writeGauge(gauge)).isEmpty(); } @Test void writeGaugeWithTimeGauge() { AtomicReference obj = new AtomicReference<>(1d); - meterNameEventTypeEnabledRegistry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); - TimeGauge timeGauge = meterNameEventTypeEnabledRegistry.find("my.timeGauge").timeGauge(); - Stream streamResult = meterNameEventTypeEnabledRegistry.writeTimeGauge(timeGauge); - assertThat(streamResult).contains("{\"eventType\":\"myTimeGauge\",\"value\":1,\"timeUnit\":\"seconds\"}"); - registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); - timeGauge = registry.find("my.timeGauge").timeGauge(); - streamResult = registry.writeTimeGauge(timeGauge); - assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"timeUnit\":\"seconds\",\"metricName\":\"myTimeGauge\",\"metricType\":\"GAUGE\"}"); - } + TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); + //test Http clientProvider + Stream streamResult = getHttpClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge); + assertThat(streamResult).contains("{\"eventType\":\"myTimeGauge\",\"value\":1000,\"timeUnit\":\"milliseconds\"}"); + + streamResult = getHttpClientProvider(httpConfig).writeTimeGauge(timeGauge); + assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"value\":1000,\"timeUnit\":\"milliseconds\",\"metricName\":\"myTimeGauge\",\"metricType\":\"GAUGE\"}"); + //test Agent clientProvider + Map result = getAgentClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge); + assertThat(result).hasSize(2); + assertThat(result).containsEntry("timeUnit", "milliseconds"); + assertThat(result).containsEntry("value", 1000); + + result = getAgentClientProvider(agentConfig).writeTimeGauge(timeGauge); + assertThat(result).hasSize(4); + assertThat(result).containsEntry("metricName", "myTimeGauge"); + assertThat(result).containsEntry("metricType", "GAUGE"); + assertThat(result).containsEntry("timeUnit", "milliseconds"); + assertThat(result).containsEntry("value", 1000); + } + @Test void writeGaugeWithTimeGaugeShouldDropNanValue() { AtomicReference obj = new AtomicReference<>(Double.NaN); - meterNameEventTypeEnabledRegistry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); - TimeGauge timeGauge = meterNameEventTypeEnabledRegistry.find("my.timeGauge").timeGauge(); - assertThat(meterNameEventTypeEnabledRegistry.writeTimeGauge(timeGauge)).isEmpty(); - registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); - timeGauge = registry.find("my.timeGauge").timeGauge(); - assertThat(registry.writeTimeGauge(timeGauge)).isEmpty(); + TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); + //test Http clientProvider + assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); + assertThat(getHttpClientProvider(httpConfig).writeTimeGauge(timeGauge)).isEmpty(); + + //test Agent clientProvider + assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); + assertThat(getAgentClientProvider(agentConfig).writeTimeGauge(timeGauge)).isEmpty(); } @Test void writeGaugeWithTimeGaugeShouldDropInfiniteValues() { AtomicReference obj = new AtomicReference<>(Double.POSITIVE_INFINITY); - meterNameEventTypeEnabledRegistry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); - TimeGauge timeGauge = meterNameEventTypeEnabledRegistry.find("my.timeGauge").timeGauge(); - assertThat(meterNameEventTypeEnabledRegistry.writeTimeGauge(timeGauge)).isEmpty(); - - obj = new AtomicReference<>(Double.NEGATIVE_INFINITY); - meterNameEventTypeEnabledRegistry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); - timeGauge = meterNameEventTypeEnabledRegistry.find("my.timeGauge").timeGauge(); - assertThat(meterNameEventTypeEnabledRegistry.writeTimeGauge(timeGauge)).isEmpty(); - - obj = new AtomicReference<>(Double.POSITIVE_INFINITY); registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); - timeGauge = registry.find("my.timeGauge").timeGauge(); - assertThat(registry.writeTimeGauge(timeGauge)).isEmpty(); + TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); + //test Http clientProvider + assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); + assertThat(getHttpClientProvider(httpConfig).writeTimeGauge(timeGauge)).isEmpty(); + + //test Agent clientProvider + assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); + assertThat(getAgentClientProvider(agentConfig).writeTimeGauge(timeGauge)).isEmpty(); obj = new AtomicReference<>(Double.NEGATIVE_INFINITY); registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); timeGauge = registry.find("my.timeGauge").timeGauge(); - assertThat(registry.writeTimeGauge(timeGauge)).isEmpty(); + //test Http clientProvider + assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); + assertThat(getHttpClientProvider(httpConfig).writeTimeGauge(timeGauge)).isEmpty(); + + //test Agent clientProvider + assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); + assertThat(getAgentClientProvider(agentConfig).writeTimeGauge(timeGauge)).isEmpty(); } @Test void writeCounterWithFunctionCounter() { - FunctionCounter counter = FunctionCounter.builder("myCounter", 1d, Number::doubleValue).register(meterNameEventTypeEnabledRegistry); - clock.add(config.step()); - Stream streamResult = meterNameEventTypeEnabledRegistry.writeFunctionCounter(counter); - assertThat(streamResult).contains("{\"eventType\":\"myCounter\",\"throughput\":1}"); - - counter = FunctionCounter.builder("myCounter", 1d, Number::doubleValue).register(registry); - clock.add(config.step()); - streamResult = registry.writeFunctionCounter(counter); + FunctionCounter counter = FunctionCounter.builder("myCounter", 1d, Number::doubleValue).register(registry); + clock.add(agentConfig.step()); + //test Http clientProvider + Stream streamResult = getHttpClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter); + assertThat(streamResult).contains("{\"eventType\":\"myCounter\",\"throughput\":1}"); + + streamResult = getHttpClientProvider(httpConfig).writeFunctionCounter(counter); assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"throughput\":1,\"metricName\":\"myCounter\",\"metricType\":\"COUNTER\"}"); + + //test Agent clientProvider + Map result = getAgentClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter); + assertThat(result).hasSize(1); + assertThat(result).containsEntry("throughput", 1); + + result = getAgentClientProvider(agentConfig).writeFunctionCounter(counter); + assertThat(result).hasSize(3); + assertThat(result).containsEntry("metricName", "myCounter"); + assertThat(result).containsEntry("metricType", "COUNTER"); + assertThat(result).containsEntry("throughput", 1); } @Test void writeCounterWithFunctionCounterShouldDropInfiniteValues() { - FunctionCounter counter = FunctionCounter.builder("myCounter", Double.POSITIVE_INFINITY, Number::doubleValue).register(meterNameEventTypeEnabledRegistry); - clock.add(config.step()); - assertThat(meterNameEventTypeEnabledRegistry.writeFunctionCounter(counter)).isEmpty(); - - counter = FunctionCounter.builder("myCounter", Double.NEGATIVE_INFINITY, Number::doubleValue).register(meterNameEventTypeEnabledRegistry); - clock.add(config.step()); - assertThat(meterNameEventTypeEnabledRegistry.writeFunctionCounter(counter)).isEmpty(); - - counter = FunctionCounter.builder("myCounter", Double.POSITIVE_INFINITY, Number::doubleValue).register(registry); - clock.add(config.step()); - assertThat(registry.writeFunctionCounter(counter)).isEmpty(); - + FunctionCounter counter = FunctionCounter.builder("myCounter", Double.POSITIVE_INFINITY, Number::doubleValue).register(registry); + clock.add(agentConfig.step()); + //test Http clientProvider + assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter)).isEmpty(); + assertThat(getHttpClientProvider(httpConfig).writeFunctionCounter(counter)).isEmpty(); + + //test Agent clientProvider + assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter)).isEmpty(); + assertThat(getAgentClientProvider(agentConfig).writeFunctionCounter(counter)).isEmpty(); + counter = FunctionCounter.builder("myCounter", Double.NEGATIVE_INFINITY, Number::doubleValue).register(registry); - clock.add(config.step()); - assertThat(registry.writeFunctionCounter(counter)).isEmpty(); + clock.add(httpConfig.step()); + //test Http clientProvider + assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter)).isEmpty(); + assertThat(getHttpClientProvider(httpConfig).writeFunctionCounter(counter)).isEmpty(); + + //test Agent clientProvider + assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter)).isEmpty(); + assertThat(getAgentClientProvider(agentConfig).writeFunctionCounter(counter)).isEmpty(); } @Test @@ -229,11 +296,14 @@ void writeMeterWhenCustomMeterHasOnlyNonFiniteValuesShouldNotBeWritten() { Measurement measurement2 = new Measurement(() -> Double.NEGATIVE_INFINITY, Statistic.VALUE); Measurement measurement3 = new Measurement(() -> Double.NaN, Statistic.VALUE); List measurements = Arrays.asList(measurement1, measurement2, measurement3); - Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(this.meterNameEventTypeEnabledRegistry); - assertThat(meterNameEventTypeEnabledRegistry.writeMeter(meter)).isEmpty(); - - meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(this.registry); - assertThat(registry.writeMeter(meter)).isEmpty(); + Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(registry); + //test Http clientProvider + assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeMeter(meter)).isEmpty(); + assertThat(getHttpClientProvider(httpConfig).writeMeter(meter)).isEmpty(); + + //test Agent clientProvider + assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeMeter(meter)).isEmpty(); + assertThat(getAgentClientProvider(agentConfig).writeMeter(meter)).isEmpty(); } @Test @@ -243,11 +313,24 @@ void writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonF Measurement measurement3 = new Measurement(() -> Double.NaN, Statistic.VALUE); Measurement measurement4 = new Measurement(() -> 1d, Statistic.VALUE); List measurements = Arrays.asList(measurement1, measurement2, measurement3, measurement4); - Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(this.meterNameEventTypeEnabledRegistry); - assertThat(meterNameEventTypeEnabledRegistry.writeMeter(meter)).contains("{\"eventType\":\"myMeter\",\"value\":1}"); - - meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(this.registry); - assertThat(registry.writeMeter(meter)).contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myMeter\",\"metricType\":\"GAUGE\"}"); + Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(registry); + //test Http clientProvider + Stream streamResult = getHttpClientProvider(meterNameEventTypeEnabledConfig).writeMeter(meter); + assertThat(streamResult).contains("{\"eventType\":\"myMeter\",\"value\":1}"); + + streamResult = getHttpClientProvider(httpConfig).writeMeter(meter); + assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myMeter\",\"metricType\":\"GAUGE\"}"); + + //test Agent clientProvider + Map result = getAgentClientProvider(meterNameEventTypeEnabledConfig).writeMeter(meter); + assertThat(result).hasSize(1); + assertThat(result).containsEntry("value", 1); + + result = getAgentClientProvider(agentConfig).writeMeter(meter); + assertThat(result).hasSize(3); + assertThat(result).containsEntry("metricName", "myMeter"); + assertThat(result).containsEntry("metricType", "GAUGE"); + assertThat(result).containsEntry("value", 1); } @Test @@ -257,26 +340,171 @@ void writeMeterWhenCustomMeterHasDuplicatesKeysShouldWriteOnlyLastValue() { Measurement measurement3 = new Measurement(() -> 2d, Statistic.VALUE); List measurements = Arrays.asList(measurement1, measurement2, measurement3); Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(this.registry); - assertThat(registry.writeMeter(meter)).contains("{\"eventType\":\"MicrometerSample\",\"value\":2,\"metricName\":\"myMeter\",\"metricType\":\"GAUGE\"}"); + //test Http clientProvider + Stream streamResult = getHttpClientProvider(httpConfig).writeMeter(meter); + assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"value\":2,\"metricName\":\"myMeter\",\"metricType\":\"GAUGE\"}"); + + //test Agent clientProvider + Map result = getAgentClientProvider(agentConfig).writeMeter(meter); + assertThat(result).hasSize(3); + assertThat(result).containsEntry("metricName", "myMeter"); + assertThat(result).containsEntry("metricType", "GAUGE"); + assertThat(result).containsEntry("value", 2); } @Test - void publish() { - MockHttpSender mockHttpSender = new MockHttpSender(); - NewRelicMeterRegistry registry = new NewRelicMeterRegistry(config, clock, new NamedThreadFactory("new-relic-test"), mockHttpSender); + void sendEventsWithHttpProvider() { + //test meterNameEventTypeEnabledConfig = false (default) + MockHttpSender mockHttpClient = new MockHttpSender(); + NewRelicHttpClientProviderImpl httpProvider = new NewRelicHttpClientProviderImpl( + httpConfig, mockHttpClient, registry.config().namingConvention(), registry.getBaseTimeUnit()); + + NewRelicMeterRegistry registry = new NewRelicMeterRegistry(httpConfig, httpProvider, clock); + + registry.gauge("my.gauge", 1d); + Gauge gauge = registry.find("my.gauge").gauge(); + + httpProvider.sendEvents(gauge.getId(), httpProvider.writeGauge(gauge)); + + assertThat(new String(mockHttpClient.getRequest().getEntity())) + .contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\"}"); + + //test meterNameEventTypeEnabledConfig = true + mockHttpClient = new MockHttpSender(); + httpProvider = new NewRelicHttpClientProviderImpl( + meterNameEventTypeEnabledConfig, mockHttpClient, registry.config().namingConvention(), registry.getBaseTimeUnit()); + + registry.gauge("my.gauge2", 1d); + gauge = registry.find("my.gauge2").gauge(); + + httpProvider.sendEvents(gauge.getId(), httpProvider.writeGauge(gauge)); + + assertThat(new String(mockHttpClient.getRequest().getEntity())) + .contains("{\"eventType\":\"myGauge2\",\"value\":1}"); + } + + @Test + void sendEventsWithAgentProvider() { + //test meterNameEventTypeEnabledConfig = false (default) + MockNewRelicAgent mockNewRelicAgent = new MockNewRelicAgent(); + NewRelicAgentClientProviderImpl agentProvider = new NewRelicAgentClientProviderImpl( + agentConfig, mockNewRelicAgent, registry.config().namingConvention(), registry.getBaseTimeUnit()); + + NewRelicMeterRegistry registry = new NewRelicMeterRegistry(agentConfig, agentProvider, clock); + + registry.gauge("my.gauge", 1d); + Gauge gauge = registry.find("my.gauge").gauge(); + + agentProvider.sendEvents(gauge.getId(), agentProvider.writeGauge(gauge)); + + assertThat(((MockNewRelicInsights)mockNewRelicAgent.getInsights()).getInsightData().getEventType()).isEqualTo("MicrometerSample"); + Map result = ((MockNewRelicInsights)mockNewRelicAgent.getInsights()).getInsightData().getAttributes(); + assertThat(result).hasSize(3); + + //test meterNameEventTypeEnabledConfig = true + mockNewRelicAgent = new MockNewRelicAgent(); + agentProvider = new NewRelicAgentClientProviderImpl( + meterNameEventTypeEnabledConfig, mockNewRelicAgent, registry.config().namingConvention(), registry.getBaseTimeUnit()); + + registry.gauge("my.gauge2", 1d); + gauge = registry.find("my.gauge2").gauge(); + + agentProvider.sendEvents(gauge.getId(), agentProvider.writeGauge(gauge)); + + assertThat(((MockNewRelicInsights)mockNewRelicAgent.getInsights()).getInsightData().getEventType()).isEqualTo("myGauge2"); + result = ((MockNewRelicInsights)mockNewRelicAgent.getInsights()).getInsightData().getAttributes(); + assertThat(result).hasSize(1); + } + + @Test + void publishWithHttpClientProvider() { + //test meterNameEventTypeEnabledConfig = false (default) + MockHttpSender mockHttpClient = new MockHttpSender(); + NewRelicHttpClientProviderImpl httpProvider = new NewRelicHttpClientProviderImpl( + httpConfig, mockHttpClient, registry.config().namingConvention(), registry.getBaseTimeUnit()); + + NewRelicMeterRegistry registry = new NewRelicMeterRegistry(httpConfig, httpProvider, clock); registry.gauge("my.gauge", 1d); Gauge gauge = registry.find("my.gauge").gauge(); assertThat(gauge).isNotNull(); registry.publish(); - - assertThat(new String(mockHttpSender.getRequest().getEntity())) - .contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\"}"); + + assertThat(new String(mockHttpClient.getRequest().getEntity())) + .contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\"}"); } @Test - void configMissingEventType() { + void publishWithAgentClientProvider() { + //test meterNameEventTypeEnabledConfig = false (default) + MockNewRelicAgent mockNewRelicAgent = new MockNewRelicAgent(); + NewRelicAgentClientProviderImpl agentProvider = new NewRelicAgentClientProviderImpl( + agentConfig, mockNewRelicAgent, registry.config().namingConvention(), registry.getBaseTimeUnit()); + + NewRelicMeterRegistry registry = new NewRelicMeterRegistry(agentConfig, agentProvider, clock); + + registry.gauge("my.gauge", 1d); + Gauge gauge = registry.find("my.gauge").gauge(); + assertThat(gauge).isNotNull(); + + registry.publish(); + + assertThat(((MockNewRelicInsights)mockNewRelicAgent.getInsights()).getInsightData().getEventType()).isEqualTo("MicrometerSample"); + Map result = ((MockNewRelicInsights)mockNewRelicAgent.getInsights()).getInsightData().getAttributes(); + assertThat(result).hasSize(3); + } + + @Test + void failsConfigMissingClientProvider() { + NewRelicConfig config = new NewRelicConfig() { + @Override + public String get(String key) { + return null; + } + }; + + @SuppressWarnings("deprecation") + Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { + new NewRelicMeterRegistry(config, null, new NewRelicNamingConvention(), clock, new NamedThreadFactory("test")); + }); + assertThat(exception.getMessage()).contains("clientProvider"); + } + + @Test + void failsConfigMissingNamingConvention() { + NewRelicConfig config = new NewRelicConfig() { + @Override + public String get(String key) { + return null; + } + }; + + @SuppressWarnings("deprecation") + Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { + new NewRelicMeterRegistry(config, new MockClientProvider(), null, clock, new NamedThreadFactory("test")); + }); + assertThat(exception.getMessage()).contains("namingConvention"); + } + + @Test + void failsConfigMissingThreadFactory() { + NewRelicConfig config = new NewRelicConfig() { + @Override + public String get(String key) { + return null; + } + }; + + @SuppressWarnings("deprecation") + Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { + new NewRelicMeterRegistry(config, new MockClientProvider(), new NewRelicNamingConvention(), clock, null); + }); + assertThat(exception.getMessage()).contains("threadFactory"); + } + + @Test + void failsConfigHttpMissingEventType() { NewRelicConfig config = new NewRelicConfig() { @Override public String eventType() { @@ -289,13 +517,41 @@ public String get(String key) { }; Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - new NewRelicMeterRegistry(config, clock); + getHttpClientProvider(config); }); assertThat(exception.getMessage()).contains("eventType"); } + + @Test + void succeedsConfigHttpMissingEventType() { + NewRelicConfig config = new NewRelicConfig() { + @Override + public boolean meterNameEventTypeEnabled() { + return true; + } + @Override + public String eventType() { + return ""; + } + @Override + public String accountId() { + return "accountId"; + } + @Override + public String apiKey() { + return "apiKey"; + } + @Override + public String get(String key) { + return null; + } + }; + + assertThat( getHttpClientProvider(config) ).isNotNull(); + } @Test - void configMissingAccountId() { + void failsConfigHttpMissingAccountId() { NewRelicConfig config = new NewRelicConfig() { @Override public String eventType() { @@ -312,13 +568,13 @@ public String get(String key) { }; Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - new NewRelicMeterRegistry(config, clock); + getHttpClientProvider(config); }); assertThat(exception.getMessage()).contains("accountId"); } @Test - void configMissingApiKey() { + void failsConfigHttpMissingApiKey() { NewRelicConfig config = new NewRelicConfig() { @Override public String eventType() { @@ -339,13 +595,13 @@ public String get(String key) { }; Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - new NewRelicMeterRegistry(config, clock); + getHttpClientProvider(config); }); assertThat(exception.getMessage()).contains("apiKey"); } @Test - void configMissingUri() { + void failsConfigHttpMissingUri() { NewRelicConfig config = new NewRelicConfig() { @Override public String eventType() { @@ -370,11 +626,50 @@ public String get(String key) { }; Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - new NewRelicMeterRegistry(config, clock); + getHttpClientProvider(config); }); assertThat(exception.getMessage()).contains("uri"); } + @Test + void failsConfigAgentMissingEventType() { + NewRelicConfig config = new NewRelicConfig() { + @Override + public String eventType() { + return ""; + } + @Override + public String get(String key) { + return null; + } + }; + + Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { + getAgentClientProvider(config); + }); + assertThat(exception.getMessage()).contains("eventType"); + } + + @Test + void succeedsConfigAgentMissingEventType() { + NewRelicConfig config = new NewRelicConfig() { + @Override + public boolean meterNameEventTypeEnabled() { + return true; + } + @Override + public String eventType() { + return ""; + } + @Override + public String get(String key) { + return null; + } + }; + + assertThat( getAgentClientProvider(config) ).isNotNull(); + } + class MockHttpSender implements HttpSender { private Request request; @@ -390,4 +685,158 @@ public Request getRequest() { } } + class MockClientProvider implements NewRelicClientProvider { + + @Override + public void publish(MeterRegistry meterRegistry, List meters) { + //No-op + } + + @Override + public Object writeFunctionTimer(FunctionTimer timer) { + //No-op + return null; + } + + @Override + public Object writeTimer(Timer timer) { + //No-op + return null; + } + + @Override + public Object writeSummary(DistributionSummary summary) { + //No-op + return null; + } + + @Override + public Object writeLongTaskTimer(LongTaskTimer timer) { + //No-op + return null; + } + + @Override + public Object writeTimeGauge(TimeGauge gauge) { + //No-op + return null; + } + + @Override + public Object writeGauge(Gauge gauge) { + //No-op + return null; + } + + @Override + public Object writeCounter(Counter counter) { + //No-op + return null; + } + + @Override + public Object writeFunctionCounter(FunctionCounter counter) { + //No-op + return null; + } + + @Override + public Object writeMeter(Meter meter) { + //No-op + return null; + } + + } + + class MockNewRelicAgent implements Agent { + + private final Insights insights; + + public MockNewRelicAgent() { + this.insights=new MockNewRelicInsights(); + } + + @Override + public Config getConfig() { + //No-op + return null; + } + + @Override + public Insights getInsights() { + return insights; + } + + public class MockNewRelicInsights implements Insights { + + private InsightData insightData; + + public InsightData getInsightData() { + return insightData; + } + + @Override + public void recordCustomEvent(String eventType, Map attributes) { + this.insightData = new InsightData(eventType, attributes); + } + + public void setInsightData(InsightData insightData) { + this.insightData = insightData; + } + + class InsightData { + private String eventType; + private Map attributes; + + public InsightData(String eventType, Map attributes) { + this.eventType=eventType; + this.attributes=attributes; + } + + public String getEventType() { + return eventType; + } + public Map getAttributes() { + return attributes; + } + } + + } + + @Override + public Logger getLogger() { + //No-op + return null; + } + + @Override + public MetricAggregator getMetricAggregator() { + //No-op + return null; + } + + @Override + public TracedMethod getTracedMethod() { + //No-op + return null; + } + + @Override + public Transaction getTransaction() { + //No-op + return null; + } + + @Override + public Map getLinkingMetadata() { + //No-op + return null; + } + + @Override + public TraceMetadata getTraceMetadata() { + //No-op + return null; + } + } } From e0029a7b3b30da0486816812d33ad0c07e7c33b3 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Sun, 6 Oct 2019 22:23:56 -0500 Subject: [PATCH 04/25] re-apply builder() and start() --- .../newrelic/NewRelicMeterRegistry.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java index 3863891cdd..6bd0624d02 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java @@ -18,11 +18,16 @@ import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.micrometer.core.instrument.Clock; import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; import io.micrometer.core.instrument.config.NamingConvention; import io.micrometer.core.instrument.step.StepMeterRegistry; import io.micrometer.core.instrument.util.NamedThreadFactory; +import io.micrometer.core.instrument.util.TimeUtils; +import io.micrometer.newrelic.OldNewRelicMeterRegistry.Builder; /** * Publishes metrics to New Relic Insights based on provider selected. @@ -34,7 +39,9 @@ public class NewRelicMeterRegistry extends StepMeterRegistry { private static final ThreadFactory DEFAULT_THREAD_FACTORY = new NamedThreadFactory("new-relic-metrics-publisher"); + private final NewRelicConfig config; private NewRelicClientProvider clientProvider; + private final Logger logger = LoggerFactory.getLogger(NewRelicMeterRegistry.class); /** * @param config Configuration options for the registry that are describable as properties. @@ -76,11 +83,24 @@ public NewRelicMeterRegistry(NewRelicConfig config, NewRelicClientProvider clien throw new MissingRequiredConfigurationException("threadFactory must be set to report metrics to New Relic"); } + this.config = config; this.clientProvider = clientProvider; config().namingConvention(namingConvention); start(threadFactory); } + + public static Builder builder(NewRelicConfig config) { + return new Builder(config); + } + + @Override + public void start(ThreadFactory threadFactory) { + if (config.enabled()) { + logger.info("publishing metrics to new relic every " + TimeUtils.format(config.step())); + } + super.start(threadFactory); + } @Override protected void publish() { From b26f516d3204e1754d5e0f3294bdd9fa2c7cde5c Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Sun, 6 Oct 2019 22:40:05 -0500 Subject: [PATCH 05/25] Comments --- .../newrelic/NewRelicHttpClientProviderImpl.java | 5 ++--- .../java/io/micrometer/newrelic/NewRelicMeterRegistry.java | 7 +++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java index 4a7a185971..5e133a1d6b 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java @@ -223,7 +223,8 @@ public Stream writeMeter(Meter meter) { private String event(Meter.Id id, Attribute... attributes) { if (!config.meterNameEventTypeEnabled()) { - //Include these if NOT generating an event per Meter/metric name + // Include contextual attributes when publishing all metrics under a single categorical eventType, + // NOT when publishing an eventType per Meter/metric name int size = attributes.length; Attribute[] newAttrs = Arrays.copyOf(attributes, size + 2); @@ -233,7 +234,6 @@ private String event(Meter.Id id, Attribute... attributes) { return event(id, Tags.empty(), newAttrs); } - return event(id, Tags.empty(), attributes); } @@ -261,7 +261,6 @@ private String event(Meter.Id id, Iterable extraTags, Attribute... attribut } void sendEvents(Meter.Id id, Object attributesObj) { - if (attributesObj instanceof Stream) { @SuppressWarnings("unchecked") Stream events = (Stream)attributesObj; diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java index 6bd0624d02..14b39f5304 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java @@ -27,10 +27,10 @@ import io.micrometer.core.instrument.step.StepMeterRegistry; import io.micrometer.core.instrument.util.NamedThreadFactory; import io.micrometer.core.instrument.util.TimeUtils; -import io.micrometer.newrelic.OldNewRelicMeterRegistry.Builder; /** - * Publishes metrics to New Relic Insights based on provider selected. + * Publishes metrics to New Relic Insights based on client provider selected (Http or Java Agent). + * Defaults to Http client provider. * * @author Jon Schneider * @author Johnny Lim @@ -122,7 +122,6 @@ public static class Builder { Builder(NewRelicConfig config) { this.config = config; - httpClientProvider(); } public Builder agentClientProvider() { @@ -133,7 +132,7 @@ public Builder httpClientProvider() { return clientProvider(new NewRelicHttpClientProviderImpl(config)); } - public Builder clientProvider(NewRelicClientProvider clientProvider) { + Builder clientProvider(NewRelicClientProvider clientProvider) { this.clientProvider = clientProvider; return this; } From 53adfabe0522240b0913d591579399dcf5eb0839 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Mon, 14 Oct 2019 20:56:36 -0500 Subject: [PATCH 06/25] Default meter registry Builder to Http/Rest client --- .../java/io/micrometer/newrelic/NewRelicMeterRegistry.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java index 14b39f5304..242937aba2 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java @@ -30,7 +30,7 @@ /** * Publishes metrics to New Relic Insights based on client provider selected (Http or Java Agent). - * Defaults to Http client provider. + * Defaults to the HTTP/REST client provider. * * @author Jon Schneider * @author Johnny Lim @@ -48,6 +48,7 @@ public class NewRelicMeterRegistry extends StepMeterRegistry { * @param clock The clock to use for timings. */ public NewRelicMeterRegistry(NewRelicConfig config, Clock clock) { + //default to the HTTP/REST client this(config, new NewRelicHttpClientProviderImpl(config), clock); } @@ -153,6 +154,10 @@ public Builder threadFactory(ThreadFactory threadFactory) { } public NewRelicMeterRegistry build() { + if (clientProvider == null) { + //default to the HTTP/REST client + clientProvider = new NewRelicHttpClientProviderImpl(config); + } return new NewRelicMeterRegistry(config, clientProvider, convention, clock, threadFactory); } } From 1107dc0a0bd96cc88ed7b3adcf65215f8b26f4f3 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Mon, 14 Oct 2019 21:06:34 -0500 Subject: [PATCH 07/25] use registry.getBaseTimeUnit() --- .../newrelic/NewRelicAgentClientProviderImpl.java | 13 +++++++------ .../newrelic/NewRelicClientProvider.java | 3 +-- .../newrelic/NewRelicHttpClientProviderImpl.java | 13 +++++++------ .../newrelic/NewRelicMeterRegistryTest.java | 15 +++++++-------- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java index 431fd086e4..778be11e9b 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java @@ -34,7 +34,6 @@ import io.micrometer.core.instrument.LongTaskTimer; import io.micrometer.core.instrument.Measurement; import io.micrometer.core.instrument.Meter; -import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.TimeGauge; import io.micrometer.core.instrument.Timer; @@ -54,13 +53,13 @@ public class NewRelicAgentClientProviderImpl implements NewRelicClientProvider { private final Agent newRelicAgent; private final NewRelicConfig config; private final NamingConvention namingConvention; - private final TimeUnit timeUnit; + private TimeUnit timeUnit = TimeUnit.MILLISECONDS; public NewRelicAgentClientProviderImpl(NewRelicConfig config) { - this(config, NewRelic.getAgent(), new NewRelicNamingConvention(), TimeUnit.MILLISECONDS); + this(config, NewRelic.getAgent(), new NewRelicNamingConvention()); } - public NewRelicAgentClientProviderImpl(NewRelicConfig config, Agent newRelicAgent, NamingConvention namingConvention, TimeUnit timeUnit) { + public NewRelicAgentClientProviderImpl(NewRelicConfig config, Agent newRelicAgent, NamingConvention namingConvention) { if (config.meterNameEventTypeEnabled() == false && StringUtils.isEmpty(config.eventType())) { @@ -70,7 +69,6 @@ public NewRelicAgentClientProviderImpl(NewRelicConfig config, Agent newRelicAgen this.newRelicAgent = newRelicAgent; this.config = config; this.namingConvention = namingConvention; - this.timeUnit = timeUnit; } @Override @@ -216,7 +214,10 @@ void addAttribute(String key, String value, Map attributes) { } @Override - public void publish(MeterRegistry meterRegistry, List meters) { + public void publish(NewRelicMeterRegistry meterRegistry, List meters) { + + this.timeUnit = meterRegistry.getBaseTimeUnit(); + // New Relic's Java Agent Insights API is backed by a reservoir/buffer // and handles the actual publishing of events to New Relic. // 1:1 mapping between Micrometer meters and New Relic events diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java index 52e4bf2d4b..709e26f0e1 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java @@ -24,7 +24,6 @@ import io.micrometer.core.instrument.Gauge; import io.micrometer.core.instrument.LongTaskTimer; import io.micrometer.core.instrument.Meter; -import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.TimeGauge; import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.config.NamingConvention; @@ -66,7 +65,7 @@ default String getEventType(Meter.Id id, NewRelicConfig config, NamingConvention return eventType; } - void publish(MeterRegistry meterRegistry, List meters); + void publish(NewRelicMeterRegistry meterRegistry, List meters); Object writeFunctionTimer(FunctionTimer timer); diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java index 5e133a1d6b..a70516086d 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java @@ -37,7 +37,6 @@ import io.micrometer.core.instrument.LongTaskTimer; import io.micrometer.core.instrument.Measurement; import io.micrometer.core.instrument.Meter; -import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.TimeGauge; @@ -64,14 +63,14 @@ public class NewRelicHttpClientProviderImpl implements NewRelicClientProvider { private final HttpSender httpClient; private final String insightsEndpoint; private final NamingConvention namingConvention; - private final TimeUnit timeUnit; + private TimeUnit timeUnit = TimeUnit.MILLISECONDS; @SuppressWarnings("deprecation") public NewRelicHttpClientProviderImpl(NewRelicConfig config) { - this(config, new HttpUrlConnectionSender(config.connectTimeout(), config.readTimeout()), new NewRelicNamingConvention(), TimeUnit.MILLISECONDS); + this(config, new HttpUrlConnectionSender(config.connectTimeout(), config.readTimeout()), new NewRelicNamingConvention()); } - public NewRelicHttpClientProviderImpl(NewRelicConfig config, HttpSender httpClient, NamingConvention namingConvention, TimeUnit timeUnit) { + public NewRelicHttpClientProviderImpl(NewRelicConfig config, HttpSender httpClient, NamingConvention namingConvention) { if (config.meterNameEventTypeEnabled() == false && (config.eventType() == null || config.eventType().isEmpty())) { @@ -90,12 +89,14 @@ public NewRelicHttpClientProviderImpl(NewRelicConfig config, HttpSender httpClie this.config = config; this.httpClient = httpClient; this.namingConvention = namingConvention; - this.timeUnit = timeUnit; this.insightsEndpoint = config.uri() + "/v1/accounts/" + config.accountId() + "/events"; } @Override - public void publish(MeterRegistry meterRegistry, List meters) { + public void publish(NewRelicMeterRegistry meterRegistry, List meters) { + + this.timeUnit = meterRegistry.getBaseTimeUnit(); + // New Relic's Insights API limits us to 1000 events per call // 1:1 mapping between Micrometer meters and New Relic events for (List batch : MeterPartition.partition(meterRegistry, Math.min(config.batchSize(), 1000))) { diff --git a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java index 1b35b31d65..e101f92541 100644 --- a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java +++ b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java @@ -44,7 +44,6 @@ import io.micrometer.core.instrument.LongTaskTimer; import io.micrometer.core.instrument.Measurement; import io.micrometer.core.instrument.Meter; -import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.MockClock; import io.micrometer.core.instrument.Statistic; import io.micrometer.core.instrument.Tags; @@ -357,7 +356,7 @@ void sendEventsWithHttpProvider() { //test meterNameEventTypeEnabledConfig = false (default) MockHttpSender mockHttpClient = new MockHttpSender(); NewRelicHttpClientProviderImpl httpProvider = new NewRelicHttpClientProviderImpl( - httpConfig, mockHttpClient, registry.config().namingConvention(), registry.getBaseTimeUnit()); + httpConfig, mockHttpClient, registry.config().namingConvention()); NewRelicMeterRegistry registry = new NewRelicMeterRegistry(httpConfig, httpProvider, clock); @@ -372,7 +371,7 @@ void sendEventsWithHttpProvider() { //test meterNameEventTypeEnabledConfig = true mockHttpClient = new MockHttpSender(); httpProvider = new NewRelicHttpClientProviderImpl( - meterNameEventTypeEnabledConfig, mockHttpClient, registry.config().namingConvention(), registry.getBaseTimeUnit()); + meterNameEventTypeEnabledConfig, mockHttpClient, registry.config().namingConvention()); registry.gauge("my.gauge2", 1d); gauge = registry.find("my.gauge2").gauge(); @@ -388,7 +387,7 @@ void sendEventsWithAgentProvider() { //test meterNameEventTypeEnabledConfig = false (default) MockNewRelicAgent mockNewRelicAgent = new MockNewRelicAgent(); NewRelicAgentClientProviderImpl agentProvider = new NewRelicAgentClientProviderImpl( - agentConfig, mockNewRelicAgent, registry.config().namingConvention(), registry.getBaseTimeUnit()); + agentConfig, mockNewRelicAgent, registry.config().namingConvention()); NewRelicMeterRegistry registry = new NewRelicMeterRegistry(agentConfig, agentProvider, clock); @@ -404,7 +403,7 @@ void sendEventsWithAgentProvider() { //test meterNameEventTypeEnabledConfig = true mockNewRelicAgent = new MockNewRelicAgent(); agentProvider = new NewRelicAgentClientProviderImpl( - meterNameEventTypeEnabledConfig, mockNewRelicAgent, registry.config().namingConvention(), registry.getBaseTimeUnit()); + meterNameEventTypeEnabledConfig, mockNewRelicAgent, registry.config().namingConvention()); registry.gauge("my.gauge2", 1d); gauge = registry.find("my.gauge2").gauge(); @@ -421,7 +420,7 @@ void publishWithHttpClientProvider() { //test meterNameEventTypeEnabledConfig = false (default) MockHttpSender mockHttpClient = new MockHttpSender(); NewRelicHttpClientProviderImpl httpProvider = new NewRelicHttpClientProviderImpl( - httpConfig, mockHttpClient, registry.config().namingConvention(), registry.getBaseTimeUnit()); + httpConfig, mockHttpClient, registry.config().namingConvention()); NewRelicMeterRegistry registry = new NewRelicMeterRegistry(httpConfig, httpProvider, clock); @@ -440,7 +439,7 @@ void publishWithAgentClientProvider() { //test meterNameEventTypeEnabledConfig = false (default) MockNewRelicAgent mockNewRelicAgent = new MockNewRelicAgent(); NewRelicAgentClientProviderImpl agentProvider = new NewRelicAgentClientProviderImpl( - agentConfig, mockNewRelicAgent, registry.config().namingConvention(), registry.getBaseTimeUnit()); + agentConfig, mockNewRelicAgent, registry.config().namingConvention()); NewRelicMeterRegistry registry = new NewRelicMeterRegistry(agentConfig, agentProvider, clock); @@ -688,7 +687,7 @@ public Request getRequest() { class MockClientProvider implements NewRelicClientProvider { @Override - public void publish(MeterRegistry meterRegistry, List meters) { + public void publish(NewRelicMeterRegistry meterRegistry, List meters) { //No-op } From fe79c5632014f3d8a4a65a24cec947c9b30da58e Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Mon, 14 Oct 2019 21:08:20 -0500 Subject: [PATCH 08/25] Move publish() up for consistency --- .../NewRelicAgentClientProviderImpl.java | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java index 778be11e9b..7ba5c54069 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java @@ -70,7 +70,32 @@ public NewRelicAgentClientProviderImpl(NewRelicConfig config, Agent newRelicAgen this.config = config; this.namingConvention = namingConvention; } - + + @Override + public void publish(NewRelicMeterRegistry meterRegistry, List meters) { + + this.timeUnit = meterRegistry.getBaseTimeUnit(); + + // New Relic's Java Agent Insights API is backed by a reservoir/buffer + // and handles the actual publishing of events to New Relic. + // 1:1 mapping between Micrometer meters and New Relic events + for (Meter meter : meters) { + sendEvents( + meter.getId(), + meter.match( + this::writeGauge, + this::writeCounter, + this::writeTimer, + this::writeSummary, + this::writeLongTaskTimer, + this::writeTimeGauge, + this::writeFunctionCounter, + this::writeFunctionTimer, + this::writeMeter) + ); + } + } + @Override public Map writeLongTaskTimer(LongTaskTimer timer) { Map attributes = new HashMap(); @@ -213,31 +238,6 @@ void addAttribute(String key, String value, Map attributes) { attributes.put(namingConvention.tagKey(key), namingConvention.tagValue(value)); } - @Override - public void publish(NewRelicMeterRegistry meterRegistry, List meters) { - - this.timeUnit = meterRegistry.getBaseTimeUnit(); - - // New Relic's Java Agent Insights API is backed by a reservoir/buffer - // and handles the actual publishing of events to New Relic. - // 1:1 mapping between Micrometer meters and New Relic events - for (Meter meter : meters) { - sendEvents( - meter.getId(), - meter.match( - this::writeGauge, - this::writeCounter, - this::writeTimer, - this::writeSummary, - this::writeLongTaskTimer, - this::writeTimeGauge, - this::writeFunctionCounter, - this::writeFunctionTimer, - this::writeMeter) - ); - } - } - void sendEvents(Meter.Id id, Object attributesObj) { if (attributesObj instanceof Map) { @SuppressWarnings("unchecked") From 2dbf891fa80ad8fe51e539d951e0dfe9032ac3c9 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Wed, 16 Oct 2019 21:40:31 -0500 Subject: [PATCH 09/25] clean up --- .../main/java/io/micrometer/newrelic/NewRelicConfig.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicConfig.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicConfig.java index f37aa51eaf..7911c65245 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicConfig.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicConfig.java @@ -15,7 +15,6 @@ */ package io.micrometer.newrelic; -import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; import io.micrometer.core.instrument.step.StepRegistryConfig; /** @@ -54,18 +53,13 @@ default String eventType() { return v; } -// public interface Http extends NewRelicConfig { default String apiKey() { String v = get(prefix() + ".apiKey"); -// if (v == null) -// throw new MissingRequiredConfigurationException("apiKey must be set to report metrics to New Relic"); return v; } default String accountId() { String v = get(prefix() + ".accountId"); -// if (v == null) -// throw new MissingRequiredConfigurationException("accountId must be set to report metrics to New Relic"); return v; } @@ -78,6 +72,5 @@ default String uri() { String v = get(prefix() + ".uri"); return (v == null) ? "https://insights-collector.newrelic.com" : v; } -// } } From 52ad284ad7b40267062fbc7195908dfd1fcd73f8 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Wed, 16 Oct 2019 22:35:31 -0500 Subject: [PATCH 10/25] fix formatting build errors --- .../newrelic/NewRelicAgentClientProviderImpl.java | 2 +- .../newrelic/NewRelicClientProvider.java | 2 +- .../newrelic/NewRelicHttpClientProviderImpl.java | 14 +++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java index 7ba5c54069..6bdf172296 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java @@ -244,7 +244,7 @@ void sendEvents(Meter.Id id, Object attributesObj) { Map attributes = (Map)attributesObj; //Delegate to New Relic Java Agent - if(attributes != null && attributes.isEmpty() == false) { + if (attributes != null && attributes.isEmpty() == false) { String eventType = getEventType(id, config, namingConvention); try { newRelicAgent.getInsights().recordCustomEvent(eventType, attributes); diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java index 709e26f0e1..b12185085e 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java @@ -55,7 +55,7 @@ public interface NewRelicClientProvider { default String getEventType(Meter.Id id, NewRelicConfig config, NamingConvention namingConvention) { String eventType = null; - if(config.meterNameEventTypeEnabled()) { + if (config.meterNameEventTypeEnabled()) { //meter/metric name event type eventType = id.getConventionName(namingConvention); } else { diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java index a70516086d..60bbd72d9a 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java @@ -182,13 +182,13 @@ public Stream writeSummary(DistributionSummary summary) { @Override public Stream writeTimer(Timer timer) { return Stream.of( - event(timer.getId(), - new Attribute(COUNT, timer.count()), - new Attribute(AVG, timer.mean(timeUnit)), - new Attribute(TOTAL_TIME, timer.totalTime(timeUnit)), - new Attribute(MAX, timer.max(timeUnit)), - new Attribute(TIME_UNIT, timeUnit.name().toLowerCase()) - ) + event(timer.getId(), + new Attribute(COUNT, timer.count()), + new Attribute(AVG, timer.mean(timeUnit)), + new Attribute(TOTAL_TIME, timer.totalTime(timeUnit)), + new Attribute(MAX, timer.max(timeUnit)), + new Attribute(TIME_UNIT, timeUnit.name().toLowerCase()) + ) ); } From 72d5f9a5e64fab99b4dcaf743f31aca3a0483fca Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Wed, 16 Oct 2019 23:21:35 -0500 Subject: [PATCH 11/25] fix test formatting build errors --- .../io/micrometer/newrelic/NewRelicMeterRegistryTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java index e101f92541..a4e422170b 100644 --- a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java +++ b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java @@ -752,7 +752,7 @@ class MockNewRelicAgent implements Agent { private final Insights insights; public MockNewRelicAgent() { - this.insights=new MockNewRelicInsights(); + this.insights = new MockNewRelicInsights(); } @Override @@ -788,8 +788,8 @@ class InsightData { private Map attributes; public InsightData(String eventType, Map attributes) { - this.eventType=eventType; - this.attributes=attributes; + this.eventType = eventType; + this.attributes = attributes; } public String getEventType() { From 3e4dc7087a6d662d9793c3660f2a1cb37af98953 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Fri, 18 Oct 2019 21:35:09 -0500 Subject: [PATCH 12/25] merged in polish --- .../NewRelicAgentClientProviderImpl.java | 9 +- .../NewRelicHttpClientProviderImpl.java | 15 +- .../newrelic/NewRelicMeterRegistryTest.java | 434 +++++++++++------- 3 files changed, 277 insertions(+), 181 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java index 6bdf172296..801e229464 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java @@ -120,7 +120,7 @@ public Map writeCounter(Counter counter) { Map writeCounterValues(Meter.Id id, double count) { Map attributes = new HashMap(); if (Double.isFinite(count)) { - addAttribute(THROUGHPUT, count, attributes); //TODO Why not named Count? ..confusing output! + addAttribute(THROUGHPUT, count, attributes); //process meter's name, type and tags addMeterAsAttributes(id, attributes); } @@ -208,9 +208,10 @@ public Map writeMeter(Meter meter) { } void addMeterAsAttributes(Meter.Id id, Map attributes) { - if (config.meterNameEventTypeEnabled() == false) { - //Include these if NOT generating an event per Meter/metric name - String name = id.getConventionName(namingConvention); // same as getConventionName( meter.getId() ) + if (!config.meterNameEventTypeEnabled()) { + // Include contextual attributes when publishing all metrics under a single categorical eventType, + // NOT when publishing an eventType per Meter/metric name + String name = id.getConventionName(namingConvention); attributes.put(METRIC_NAME, name); attributes.put(METRIC_TYPE, id.getType().toString()); } diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java index 60bbd72d9a..a105631eed 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java @@ -43,8 +43,7 @@ import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; import io.micrometer.core.instrument.config.NamingConvention; -import io.micrometer.core.instrument.util.DoubleFormat; -import io.micrometer.core.instrument.util.MeterPartition; +import io.micrometer.core.instrument.util.*; import io.micrometer.core.ipc.http.HttpSender; import io.micrometer.core.ipc.http.HttpUrlConnectionSender; @@ -72,17 +71,16 @@ public NewRelicHttpClientProviderImpl(NewRelicConfig config) { public NewRelicHttpClientProviderImpl(NewRelicConfig config, HttpSender httpClient, NamingConvention namingConvention) { - if (config.meterNameEventTypeEnabled() == false - && (config.eventType() == null || config.eventType().isEmpty())) { + if (!config.meterNameEventTypeEnabled() && StringUtils.isEmpty(config.eventType())) { throw new MissingRequiredConfigurationException("eventType must be set to report metrics to New Relic"); } - if (config.accountId() == null || config.accountId().isEmpty()) { + if (StringUtils.isEmpty(config.accountId())) { throw new MissingRequiredConfigurationException("accountId must be set to report metrics to New Relic"); } - if (config.apiKey() == null || config.apiKey().isEmpty()) { + if (StringUtils.isEmpty(config.apiKey())) { throw new MissingRequiredConfigurationException("apiKey must be set to report metrics to New Relic"); } - if (config.uri() == null || config.uri().isEmpty()) { + if (StringUtils.isEmpty(config.uri())) { throw new MissingRequiredConfigurationException("uri must be set to report metrics to New Relic"); } @@ -140,7 +138,6 @@ public Stream writeFunctionCounter(FunctionCounter counter) { @Override public Stream writeCounter(Counter counter) { - //TODO: Double.isFinite() check here like writeFunctionCounter ??? return Stream.of(event(counter.getId(), new Attribute(THROUGHPUT, counter.count()))); } @@ -225,7 +222,7 @@ public Stream writeMeter(Meter meter) { private String event(Meter.Id id, Attribute... attributes) { if (!config.meterNameEventTypeEnabled()) { // Include contextual attributes when publishing all metrics under a single categorical eventType, - // NOT when publishing an eventType per Meter/metric name + // NOT when publishing an eventType per Meter/metric name int size = attributes.length; Attribute[] newAttrs = Arrays.copyOf(attributes, size + 2); diff --git a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java index a4e422170b..7111c4a1ee 100644 --- a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java +++ b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java @@ -16,14 +16,14 @@ package io.micrometer.newrelic; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; -import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -90,6 +90,7 @@ public String apiKey() { @Override public boolean meterNameEventTypeEnabled() { + // Previous behavior for backward compatibility return true; } @@ -121,172 +122,245 @@ NewRelicHttpClientProviderImpl getHttpClientProvider(NewRelicConfig config) { @Test void writeGauge() { - registry.gauge("my.gauge", 1d); - Gauge gauge = registry.find("my.gauge").gauge(); //test Http clientProvider - Stream streamResult = getHttpClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge); - assertThat(streamResult).contains("{\"eventType\":\"myGauge\",\"value\":1}"); - - streamResult = getHttpClientProvider(httpConfig).writeGauge(gauge); - assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\"}"); - - //test Agent clientProvider - Map result = getAgentClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge); - assertThat(result).hasSize(1); - assertThat(result).containsEntry("value", 1); + writeGauge(meterNameEventTypeEnabledConfig, "{\"eventType\":\"myGauge\",\"value\":1}"); + writeGauge(httpConfig, + "{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\"}"); - result = getAgentClientProvider(agentConfig).writeGauge(gauge); - assertThat(result).hasSize(3); - assertThat(result).containsEntry("metricName", "myGauge"); - assertThat(result).containsEntry("metricType", "GAUGE"); - assertThat(result).containsEntry("value", 1); + //test Agent clientProvider + Map expectedEntries = new HashMap<>(); + expectedEntries.put("value", 1); + writeGauge(meterNameEventTypeEnabledConfig, expectedEntries); + expectedEntries.put("metricName", "myGauge"); + expectedEntries.put("metricType", "GAUGE"); + writeGauge(agentConfig, expectedEntries); } + private void writeGauge(NewRelicConfig config, String expectedJson) { + registry.gauge("my.gauge", 1d); + Gauge gauge = registry.find("my.gauge").gauge(); + assertThat(getHttpClientProvider(config).writeGauge(gauge)).containsExactly(expectedJson); + } + + private void writeGauge(NewRelicConfig config, Map expectedEntries) { + registry.gauge("my.gauge", 1d); + Gauge gauge = registry.find("my.gauge").gauge(); + Map result = getAgentClientProvider(config).writeGauge(gauge); + assertThat(result).hasSize(expectedEntries.size()); + assertThat(result).containsExactlyEntriesOf(expectedEntries); + } + + @Test void writeGaugeShouldDropNanValue() { - registry.gauge("my.gauge", Double.NaN); - Gauge gauge = registry.find("my.gauge").gauge(); //test Http clientProvider - assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); - assertThat(getHttpClientProvider(httpConfig).writeGauge(gauge)).isEmpty(); + writeGaugeShouldDropNanValue(getHttpClientProvider(meterNameEventTypeEnabledConfig)); + writeGaugeShouldDropNanValue(getHttpClientProvider(httpConfig)); //test Agent clientProvider - assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); - assertThat(getAgentClientProvider(agentConfig).writeGauge(gauge)).isEmpty(); + writeGaugeShouldDropNanValue(getAgentClientProvider(meterNameEventTypeEnabledConfig)); + writeGaugeShouldDropNanValue(getAgentClientProvider(agentConfig)); + } + + private void writeGaugeShouldDropNanValue(NewRelicHttpClientProviderImpl clientProvider) { + registry.gauge("my.gauge", Double.NaN); + Gauge gauge = registry.find("my.gauge").gauge(); + assertThat(clientProvider.writeGauge(gauge)).isEmpty(); } + + private void writeGaugeShouldDropNanValue(NewRelicAgentClientProviderImpl clientProvider) { + registry.gauge("my.gauge", Double.NaN); + Gauge gauge = registry.find("my.gauge").gauge(); + assertThat(clientProvider.writeGauge(gauge)).isEmpty(); + } @Test void writeGaugeShouldDropInfiniteValues() { - registry.gauge("my.gauge", Double.POSITIVE_INFINITY); - Gauge gauge = registry.find("my.gauge").gauge(); //test Http clientProvider - assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); - assertThat(getHttpClientProvider(httpConfig).writeGauge(gauge)).isEmpty(); + writeGaugeShouldDropInfiniteValues(getHttpClientProvider(meterNameEventTypeEnabledConfig)); + writeGaugeShouldDropInfiniteValues(getHttpClientProvider(httpConfig)); //test Agent clientProvider - assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); - assertThat(getAgentClientProvider(agentConfig).writeGauge(gauge)).isEmpty(); + writeGaugeShouldDropInfiniteValues(getAgentClientProvider(meterNameEventTypeEnabledConfig)); + writeGaugeShouldDropInfiniteValues(getAgentClientProvider(agentConfig)); + } + + private void writeGaugeShouldDropInfiniteValues(NewRelicHttpClientProviderImpl clientProvider) { + registry.gauge("my.gauge", Double.POSITIVE_INFINITY); + Gauge gauge = registry.find("my.gauge").gauge(); + assertThat(clientProvider.writeGauge(gauge)).isEmpty(); registry.gauge("my.gauge", Double.NEGATIVE_INFINITY); gauge = registry.find("my.gauge").gauge(); - //test Http clientProvider - assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); - assertThat(getHttpClientProvider(httpConfig).writeGauge(gauge)).isEmpty(); - - //test Agent clientProvider - assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeGauge(gauge)).isEmpty(); - assertThat(getAgentClientProvider(agentConfig).writeGauge(gauge)).isEmpty(); + assertThat(clientProvider.writeGauge(gauge)).isEmpty(); } + + private void writeGaugeShouldDropInfiniteValues(NewRelicAgentClientProviderImpl clientProvider) { + registry.gauge("my.gauge", Double.POSITIVE_INFINITY); + Gauge gauge = registry.find("my.gauge").gauge(); + assertThat(clientProvider.writeGauge(gauge)).isEmpty(); + registry.gauge("my.gauge", Double.NEGATIVE_INFINITY); + gauge = registry.find("my.gauge").gauge(); + assertThat(clientProvider.writeGauge(gauge)).isEmpty(); + } + @Test void writeGaugeWithTimeGauge() { + //test Http clientProvider + writeGaugeWithTimeGauge(getHttpClientProvider(meterNameEventTypeEnabledConfig), + "{\"eventType\":\"myTimeGauge\",\"value\":1000,\"timeUnit\":\"milliseconds\"}"); + writeGaugeWithTimeGauge(getHttpClientProvider(httpConfig), + "{\"eventType\":\"MicrometerSample\",\"value\":1000,\"timeUnit\":\"milliseconds\",\"metricName\":\"myTimeGauge\",\"metricType\":\"GAUGE\"}"); + + //test Agent clientProvider + Map expectedEntries = new HashMap<>(); + expectedEntries.put("value", 1000); + expectedEntries.put("timeUnit", "milliseconds"); + writeGaugeWithTimeGauge(getAgentClientProvider(meterNameEventTypeEnabledConfig), expectedEntries); + expectedEntries.put("metricName", "myTimeGauge"); + expectedEntries.put("metricType", "GAUGE"); + writeGaugeWithTimeGauge(getAgentClientProvider(agentConfig), expectedEntries); + } + + private void writeGaugeWithTimeGauge(NewRelicHttpClientProviderImpl clientProvider, String expectedJson) { + AtomicReference obj = new AtomicReference<>(1d); + registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); + TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); + assertThat(clientProvider.writeTimeGauge(timeGauge)).containsExactly(expectedJson); + } + + private void writeGaugeWithTimeGauge(NewRelicAgentClientProviderImpl clientProvider, Map expectedEntries) { AtomicReference obj = new AtomicReference<>(1d); registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); + Map result = clientProvider.writeTimeGauge(timeGauge); + assertThat(result).hasSize(expectedEntries.size()); + assertThat(result).containsExactlyEntriesOf(expectedEntries); + } + + @Test + void writeGaugeWithTimeGaugeShouldDropNanValue() { //test Http clientProvider - Stream streamResult = getHttpClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge); - assertThat(streamResult).contains("{\"eventType\":\"myTimeGauge\",\"value\":1000,\"timeUnit\":\"milliseconds\"}"); + writeGaugeWithTimeGaugeShouldDropNanValue(getHttpClientProvider(meterNameEventTypeEnabledConfig)); + writeGaugeWithTimeGaugeShouldDropNanValue(getHttpClientProvider(httpConfig)); - streamResult = getHttpClientProvider(httpConfig).writeTimeGauge(timeGauge); - assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"value\":1000,\"timeUnit\":\"milliseconds\",\"metricName\":\"myTimeGauge\",\"metricType\":\"GAUGE\"}"); - //test Agent clientProvider - Map result = getAgentClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge); - assertThat(result).hasSize(2); - assertThat(result).containsEntry("timeUnit", "milliseconds"); - assertThat(result).containsEntry("value", 1000); - - result = getAgentClientProvider(agentConfig).writeTimeGauge(timeGauge); - assertThat(result).hasSize(4); - assertThat(result).containsEntry("metricName", "myTimeGauge"); - assertThat(result).containsEntry("metricType", "GAUGE"); - assertThat(result).containsEntry("timeUnit", "milliseconds"); - assertThat(result).containsEntry("value", 1000); + writeGaugeWithTimeGaugeShouldDropNanValue(getAgentClientProvider(meterNameEventTypeEnabledConfig)); + writeGaugeWithTimeGaugeShouldDropNanValue(getAgentClientProvider(agentConfig)); } - @Test - void writeGaugeWithTimeGaugeShouldDropNanValue() { + private void writeGaugeWithTimeGaugeShouldDropNanValue(NewRelicHttpClientProviderImpl clientProvider) { + AtomicReference obj = new AtomicReference<>(Double.NaN); + registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); + TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); + assertThat(clientProvider.writeTimeGauge(timeGauge)).isEmpty(); + } + + private void writeGaugeWithTimeGaugeShouldDropNanValue(NewRelicAgentClientProviderImpl clientProvider) { AtomicReference obj = new AtomicReference<>(Double.NaN); registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); + assertThat(clientProvider.writeTimeGauge(timeGauge)).isEmpty(); + } + + @Test + void writeGaugeWithTimeGaugeShouldDropInfiniteValues() { //test Http clientProvider - assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); - assertThat(getHttpClientProvider(httpConfig).writeTimeGauge(timeGauge)).isEmpty(); + writeGaugeWithTimeGaugeShouldDropInfiniteValues(getHttpClientProvider(meterNameEventTypeEnabledConfig)); + writeGaugeWithTimeGaugeShouldDropInfiniteValues(getHttpClientProvider(httpConfig)); //test Agent clientProvider - assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); - assertThat(getAgentClientProvider(agentConfig).writeTimeGauge(timeGauge)).isEmpty(); + writeGaugeWithTimeGaugeShouldDropInfiniteValues(getAgentClientProvider(meterNameEventTypeEnabledConfig)); + writeGaugeWithTimeGaugeShouldDropInfiniteValues(getAgentClientProvider(agentConfig)); } + + private void writeGaugeWithTimeGaugeShouldDropInfiniteValues(NewRelicHttpClientProviderImpl clientProvider) { + AtomicReference obj = new AtomicReference<>(Double.POSITIVE_INFINITY); + registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); + TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); + assertThat(clientProvider.writeTimeGauge(timeGauge)).isEmpty(); - @Test - void writeGaugeWithTimeGaugeShouldDropInfiniteValues() { + obj = new AtomicReference<>(Double.NEGATIVE_INFINITY); + registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); + timeGauge = registry.find("my.timeGauge").timeGauge(); + assertThat(clientProvider.writeTimeGauge(timeGauge)).isEmpty(); + } + + private void writeGaugeWithTimeGaugeShouldDropInfiniteValues(NewRelicAgentClientProviderImpl clientProvider) { AtomicReference obj = new AtomicReference<>(Double.POSITIVE_INFINITY); registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); - //test Http clientProvider - assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); - assertThat(getHttpClientProvider(httpConfig).writeTimeGauge(timeGauge)).isEmpty(); - - //test Agent clientProvider - assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); - assertThat(getAgentClientProvider(agentConfig).writeTimeGauge(timeGauge)).isEmpty(); - + assertThat(clientProvider.writeTimeGauge(timeGauge)).isEmpty(); + obj = new AtomicReference<>(Double.NEGATIVE_INFINITY); registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); timeGauge = registry.find("my.timeGauge").timeGauge(); - //test Http clientProvider - assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); - assertThat(getHttpClientProvider(httpConfig).writeTimeGauge(timeGauge)).isEmpty(); - - //test Agent clientProvider - assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeTimeGauge(timeGauge)).isEmpty(); - assertThat(getAgentClientProvider(agentConfig).writeTimeGauge(timeGauge)).isEmpty(); + assertThat(clientProvider.writeTimeGauge(timeGauge)).isEmpty(); } @Test void writeCounterWithFunctionCounter() { FunctionCounter counter = FunctionCounter.builder("myCounter", 1d, Number::doubleValue).register(registry); - clock.add(agentConfig.step()); + clock.add(httpConfig.step()); //test Http clientProvider - Stream streamResult = getHttpClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter); - assertThat(streamResult).contains("{\"eventType\":\"myCounter\",\"throughput\":1}"); - - streamResult = getHttpClientProvider(httpConfig).writeFunctionCounter(counter); - assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"throughput\":1,\"metricName\":\"myCounter\",\"metricType\":\"COUNTER\"}"); - - //test Agent clientProvider - Map result = getAgentClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter); - assertThat(result).hasSize(1); - assertThat(result).containsEntry("throughput", 1); + writeCounterWithFunctionCounter(counter, getHttpClientProvider(meterNameEventTypeEnabledConfig), + "{\"eventType\":\"myCounter\",\"throughput\":1}"); + writeCounterWithFunctionCounter(counter, getHttpClientProvider(httpConfig), + "{\"eventType\":\"MicrometerSample\",\"throughput\":1,\"metricName\":\"myCounter\",\"metricType\":\"COUNTER\"}"); - result = getAgentClientProvider(agentConfig).writeFunctionCounter(counter); - assertThat(result).hasSize(3); - assertThat(result).containsEntry("metricName", "myCounter"); - assertThat(result).containsEntry("metricType", "COUNTER"); - assertThat(result).containsEntry("throughput", 1); + //test Agent clientProvider + Map expectedEntries = new HashMap<>(); + expectedEntries.put("throughput", 1); + writeCounterWithFunctionCounter(counter, getAgentClientProvider(meterNameEventTypeEnabledConfig), expectedEntries); + expectedEntries.put("metricName", "myCounter"); + expectedEntries.put("metricType", "COUNTER"); + writeCounterWithFunctionCounter(counter, getAgentClientProvider(agentConfig), expectedEntries); } + private void writeCounterWithFunctionCounter(FunctionCounter counter, NewRelicHttpClientProviderImpl clientProvider, String expectedJson) { + assertThat(clientProvider.writeFunctionCounter(counter)).containsExactly(expectedJson); + } + + private void writeCounterWithFunctionCounter(FunctionCounter counter, NewRelicAgentClientProviderImpl clientProvider, Map expectedEntries) { + Map result = clientProvider.writeFunctionCounter(counter); + assertThat(result).hasSize(expectedEntries.size()); + assertThat(result).containsExactlyEntriesOf(expectedEntries); + } + @Test void writeCounterWithFunctionCounterShouldDropInfiniteValues() { - FunctionCounter counter = FunctionCounter.builder("myCounter", Double.POSITIVE_INFINITY, Number::doubleValue).register(registry); - clock.add(agentConfig.step()); //test Http clientProvider - assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter)).isEmpty(); - assertThat(getHttpClientProvider(httpConfig).writeFunctionCounter(counter)).isEmpty(); + writeCounterWithFunctionCounterShouldDropInfiniteValues(getHttpClientProvider(meterNameEventTypeEnabledConfig)); + writeCounterWithFunctionCounterShouldDropInfiniteValues(getHttpClientProvider(httpConfig)); //test Agent clientProvider - assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter)).isEmpty(); - assertThat(getAgentClientProvider(agentConfig).writeFunctionCounter(counter)).isEmpty(); + writeCounterWithFunctionCounterShouldDropInfiniteValues(getAgentClientProvider(meterNameEventTypeEnabledConfig)); + writeCounterWithFunctionCounterShouldDropInfiniteValues(getAgentClientProvider(agentConfig)); + } - counter = FunctionCounter.builder("myCounter", Double.NEGATIVE_INFINITY, Number::doubleValue).register(registry); + private void writeCounterWithFunctionCounterShouldDropInfiniteValues(NewRelicHttpClientProviderImpl clientProvider) { + FunctionCounter counter = FunctionCounter.builder("myCounter", Double.POSITIVE_INFINITY, Number::doubleValue) + .register(registry); clock.add(httpConfig.step()); - //test Http clientProvider - assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter)).isEmpty(); - assertThat(getHttpClientProvider(httpConfig).writeFunctionCounter(counter)).isEmpty(); - - //test Agent clientProvider - assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeFunctionCounter(counter)).isEmpty(); - assertThat(getAgentClientProvider(agentConfig).writeFunctionCounter(counter)).isEmpty(); + assertThat(clientProvider.writeFunctionCounter(counter)).isEmpty(); + + counter = FunctionCounter.builder("myCounter", Double.NEGATIVE_INFINITY, Number::doubleValue) + .register(registry); + clock.add(httpConfig.step()); + assertThat(clientProvider.writeFunctionCounter(counter)).isEmpty(); + } + + private void writeCounterWithFunctionCounterShouldDropInfiniteValues(NewRelicAgentClientProviderImpl clientProvider) { + FunctionCounter counter = FunctionCounter.builder("myCounter", Double.POSITIVE_INFINITY, Number::doubleValue) + .register(registry); + clock.add(httpConfig.step()); + assertThat(clientProvider.writeFunctionCounter(counter)).isEmpty(); + + counter = FunctionCounter.builder("myCounter", Double.NEGATIVE_INFINITY, Number::doubleValue) + .register(registry); + clock.add(httpConfig.step()); + assertThat(clientProvider.writeFunctionCounter(counter)).isEmpty(); } @Test @@ -295,14 +369,30 @@ void writeMeterWhenCustomMeterHasOnlyNonFiniteValuesShouldNotBeWritten() { Measurement measurement2 = new Measurement(() -> Double.NEGATIVE_INFINITY, Statistic.VALUE); Measurement measurement3 = new Measurement(() -> Double.NaN, Statistic.VALUE); List measurements = Arrays.asList(measurement1, measurement2, measurement3); - Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(registry); + //test Http clientProvider - assertThat(getHttpClientProvider(meterNameEventTypeEnabledConfig).writeMeter(meter)).isEmpty(); - assertThat(getHttpClientProvider(httpConfig).writeMeter(meter)).isEmpty(); + writeMeterWhenCustomMeterHasOnlyNonFiniteValuesShouldNotBeWritten( + measurements, getHttpClientProvider(meterNameEventTypeEnabledConfig)); + writeMeterWhenCustomMeterHasOnlyNonFiniteValuesShouldNotBeWritten( + measurements, getHttpClientProvider(httpConfig)); //test Agent clientProvider - assertThat(getAgentClientProvider(meterNameEventTypeEnabledConfig).writeMeter(meter)).isEmpty(); - assertThat(getAgentClientProvider(agentConfig).writeMeter(meter)).isEmpty(); + writeMeterWhenCustomMeterHasOnlyNonFiniteValuesShouldNotBeWritten( + measurements, getAgentClientProvider(meterNameEventTypeEnabledConfig)); + writeMeterWhenCustomMeterHasOnlyNonFiniteValuesShouldNotBeWritten( + measurements, getAgentClientProvider(agentConfig)); + } + + private void writeMeterWhenCustomMeterHasOnlyNonFiniteValuesShouldNotBeWritten( + List measurements, NewRelicHttpClientProviderImpl clientProvider) { + Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(registry); + assertThat(clientProvider.writeMeter(meter)).isEmpty(); + } + + private void writeMeterWhenCustomMeterHasOnlyNonFiniteValuesShouldNotBeWritten( + List measurements, NewRelicAgentClientProviderImpl clientProvider) { + Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(registry); + assertThat(clientProvider.writeMeter(meter)).isEmpty(); } @Test @@ -312,24 +402,37 @@ void writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonF Measurement measurement3 = new Measurement(() -> Double.NaN, Statistic.VALUE); Measurement measurement4 = new Measurement(() -> 1d, Statistic.VALUE); List measurements = Arrays.asList(measurement1, measurement2, measurement3, measurement4); - Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(registry); //test Http clientProvider - Stream streamResult = getHttpClientProvider(meterNameEventTypeEnabledConfig).writeMeter(meter); - assertThat(streamResult).contains("{\"eventType\":\"myMeter\",\"value\":1}"); - - streamResult = getHttpClientProvider(httpConfig).writeMeter(meter); - assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myMeter\",\"metricType\":\"GAUGE\"}"); + writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonFiniteValues( + measurements, getHttpClientProvider(meterNameEventTypeEnabledConfig), + "{\"eventType\":\"myMeter\",\"value\":1}"); + writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonFiniteValues( + measurements, getHttpClientProvider(httpConfig), + "{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myMeter\",\"metricType\":\"GAUGE\"}"); //test Agent clientProvider - Map result = getAgentClientProvider(meterNameEventTypeEnabledConfig).writeMeter(meter); - assertThat(result).hasSize(1); - assertThat(result).containsEntry("value", 1); - - result = getAgentClientProvider(agentConfig).writeMeter(meter); - assertThat(result).hasSize(3); - assertThat(result).containsEntry("metricName", "myMeter"); - assertThat(result).containsEntry("metricType", "GAUGE"); - assertThat(result).containsEntry("value", 1); + Map expectedEntries = new HashMap<>(); + expectedEntries.put("value", 1); + writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonFiniteValues( + measurements, getAgentClientProvider(meterNameEventTypeEnabledConfig), expectedEntries); + expectedEntries.put("metricName", "myMeter"); + expectedEntries.put("metricType", "GAUGE"); + writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonFiniteValues( + measurements, getAgentClientProvider(agentConfig), expectedEntries); + } + + private void writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonFiniteValues( + List measurements, NewRelicHttpClientProviderImpl clientProvider, String expectedJson) { + Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(registry); + assertThat(clientProvider.writeMeter(meter)).containsExactly(expectedJson); + } + + private void writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonFiniteValues( + List measurements, NewRelicAgentClientProviderImpl clientProvider, Map expectedEntries) { + Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(registry); + Map result = clientProvider.writeMeter(meter); + assertThat(result).hasSize(expectedEntries.size()); + assertThat(result).containsExactlyEntriesOf(expectedEntries); } @Test @@ -340,16 +443,20 @@ void writeMeterWhenCustomMeterHasDuplicatesKeysShouldWriteOnlyLastValue() { List measurements = Arrays.asList(measurement1, measurement2, measurement3); Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(this.registry); //test Http clientProvider - Stream streamResult = getHttpClientProvider(httpConfig).writeMeter(meter); - assertThat(streamResult).contains("{\"eventType\":\"MicrometerSample\",\"value\":2,\"metricName\":\"myMeter\",\"metricType\":\"GAUGE\"}"); + assertThat(getHttpClientProvider(httpConfig).writeMeter(meter)).containsExactly( + "{\"eventType\":\"MicrometerSample\",\"value\":2,\"metricName\":\"myMeter\",\"metricType\":\"GAUGE\"}"); //test Agent clientProvider + Map expectedEntries = new HashMap<>(); + expectedEntries.put("value", 2); + expectedEntries.put("metricName", "myMeter"); + expectedEntries.put("metricType", "GAUGE"); Map result = getAgentClientProvider(agentConfig).writeMeter(meter); - assertThat(result).hasSize(3); - assertThat(result).containsEntry("metricName", "myMeter"); - assertThat(result).containsEntry("metricType", "GAUGE"); - assertThat(result).containsEntry("value", 2); + assertThat(result).hasSize(expectedEntries.size()); + assertThat(result).containsExactlyEntriesOf(expectedEntries); } + + @Test void sendEventsWithHttpProvider() { @@ -463,13 +570,12 @@ public String get(String key) { } }; - @SuppressWarnings("deprecation") - Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - new NewRelicMeterRegistry(config, null, new NewRelicNamingConvention(), clock, new NamedThreadFactory("test")); - }); - assertThat(exception.getMessage()).contains("clientProvider"); + assertThatThrownBy(() -> new NewRelicMeterRegistry(config, null, clock)) + .isExactlyInstanceOf(MissingRequiredConfigurationException.class) + .hasMessageContaining("clientProvider"); } + @SuppressWarnings("deprecation") @Test void failsConfigMissingNamingConvention() { NewRelicConfig config = new NewRelicConfig() { @@ -479,13 +585,12 @@ public String get(String key) { } }; - @SuppressWarnings("deprecation") - Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - new NewRelicMeterRegistry(config, new MockClientProvider(), null, clock, new NamedThreadFactory("test")); - }); - assertThat(exception.getMessage()).contains("namingConvention"); + assertThatThrownBy(() -> new NewRelicMeterRegistry(config, new MockClientProvider(), null, clock, new NamedThreadFactory("test"))) + .isExactlyInstanceOf(MissingRequiredConfigurationException.class) + .hasMessageContaining("namingConvention"); } + @SuppressWarnings("deprecation") @Test void failsConfigMissingThreadFactory() { NewRelicConfig config = new NewRelicConfig() { @@ -495,11 +600,9 @@ public String get(String key) { } }; - @SuppressWarnings("deprecation") - Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - new NewRelicMeterRegistry(config, new MockClientProvider(), new NewRelicNamingConvention(), clock, null); - }); - assertThat(exception.getMessage()).contains("threadFactory"); + assertThatThrownBy(() -> new NewRelicMeterRegistry(config, new MockClientProvider(), new NewRelicNamingConvention(), clock, null)) + .isExactlyInstanceOf(MissingRequiredConfigurationException.class) + .hasMessageContaining("threadFactory"); } @Test @@ -515,10 +618,9 @@ public String get(String key) { } }; - Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - getHttpClientProvider(config); - }); - assertThat(exception.getMessage()).contains("eventType"); + assertThatThrownBy(() -> getHttpClientProvider(config)) + .isExactlyInstanceOf(MissingRequiredConfigurationException.class) + .hasMessageContaining("eventType"); } @Test @@ -566,10 +668,9 @@ public String get(String key) { } }; - Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - getHttpClientProvider(config); - }); - assertThat(exception.getMessage()).contains("accountId"); + assertThatThrownBy(() -> getHttpClientProvider(config)) + .isExactlyInstanceOf(MissingRequiredConfigurationException.class) + .hasMessageContaining("accountId"); } @Test @@ -593,10 +694,9 @@ public String get(String key) { } }; - Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - getHttpClientProvider(config); - }); - assertThat(exception.getMessage()).contains("apiKey"); + assertThatThrownBy(() -> getHttpClientProvider(config)) + .isExactlyInstanceOf(MissingRequiredConfigurationException.class) + .hasMessageContaining("apiKey"); } @Test @@ -624,10 +724,9 @@ public String get(String key) { } }; - Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - getHttpClientProvider(config); - }); - assertThat(exception.getMessage()).contains("uri"); + assertThatThrownBy(() -> getHttpClientProvider(config)) + .isExactlyInstanceOf(MissingRequiredConfigurationException.class) + .hasMessageContaining("uri"); } @Test @@ -643,10 +742,9 @@ public String get(String key) { } }; - Exception exception = assertThrows(MissingRequiredConfigurationException.class, () -> { - getAgentClientProvider(config); - }); - assertThat(exception.getMessage()).contains("eventType"); + assertThatThrownBy(() -> getAgentClientProvider(config)) + .isExactlyInstanceOf(MissingRequiredConfigurationException.class) + .hasMessageContaining("eventType"); } @Test From e4163ca396bb028c84755ccccf62f7e3a60edcb5 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Mon, 21 Oct 2019 20:04:39 -0500 Subject: [PATCH 13/25] Revert back to BaseTimeUnit Seconds --- .../NewRelicAgentClientProviderImpl.java | 17 ++++--- .../NewRelicHttpClientProviderImpl.java | 25 +++++----- .../newrelic/NewRelicMeterRegistry.java | 2 +- .../newrelic/NewRelicMeterRegistryTest.java | 46 +++++++++---------- 4 files changed, 44 insertions(+), 46 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java index 801e229464..f880b93465 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java @@ -53,7 +53,6 @@ public class NewRelicAgentClientProviderImpl implements NewRelicClientProvider { private final Agent newRelicAgent; private final NewRelicConfig config; private final NamingConvention namingConvention; - private TimeUnit timeUnit = TimeUnit.MILLISECONDS; public NewRelicAgentClientProviderImpl(NewRelicConfig config) { this(config, NewRelic.getAgent(), new NewRelicNamingConvention()); @@ -73,9 +72,6 @@ public NewRelicAgentClientProviderImpl(NewRelicConfig config, Agent newRelicAgen @Override public void publish(NewRelicMeterRegistry meterRegistry, List meters) { - - this.timeUnit = meterRegistry.getBaseTimeUnit(); - // New Relic's Java Agent Insights API is backed by a reservoir/buffer // and handles the actual publishing of events to New Relic. // 1:1 mapping between Micrometer meters and New Relic events @@ -99,9 +95,10 @@ public void publish(NewRelicMeterRegistry meterRegistry, List meters) { @Override public Map writeLongTaskTimer(LongTaskTimer timer) { Map attributes = new HashMap(); + TimeUnit timeUnit = TimeUnit.valueOf(timer.getId().getBaseUnit()); addAttribute(ACTIVE_TASKS, timer.activeTasks(), attributes); addAttribute(DURATION, timer.duration(timeUnit), attributes); - addAttribute(TIME_UNIT, timeUnit.name().toLowerCase(), attributes); + addAttribute(TIME_UNIT, timeUnit.toString().toLowerCase(), attributes); //process meter's name, type and tags addMeterAsAttributes(timer.getId(), attributes); return attributes; @@ -142,10 +139,10 @@ public Map writeGauge(Gauge gauge) { @Override public Map writeTimeGauge(TimeGauge gauge) { Map attributes = new HashMap(); - double value = gauge.value(timeUnit); + double value = gauge.value(); if (Double.isFinite(value)) { addAttribute(VALUE, value, attributes); - addAttribute(TIME_UNIT, gauge.baseTimeUnit().name().toLowerCase(), attributes); + addAttribute(TIME_UNIT, gauge.baseTimeUnit().toString().toLowerCase(), attributes); //process meter's name, type and tags addMeterAsAttributes(gauge.getId(), attributes); } @@ -167,11 +164,12 @@ public Map writeSummary(DistributionSummary summary) { @Override public Map writeTimer(Timer timer) { Map attributes = new HashMap(); + TimeUnit timeUnit = TimeUnit.valueOf(timer.getId().getBaseUnit()); addAttribute(COUNT, (new Double(timer.count())).longValue(), attributes); addAttribute(AVG, timer.mean(timeUnit), attributes); addAttribute(TOTAL_TIME, timer.totalTime(timeUnit), attributes); addAttribute(MAX, timer.max(timeUnit), attributes); - addAttribute(TIME_UNIT, timeUnit.name().toLowerCase(), attributes); + addAttribute(TIME_UNIT, timeUnit.toString().toLowerCase(), attributes); //process meter's name, type and tags addMeterAsAttributes(timer.getId(), attributes); return attributes; @@ -180,10 +178,11 @@ public Map writeTimer(Timer timer) { @Override public Map writeFunctionTimer(FunctionTimer timer) { Map attributes = new HashMap(); + TimeUnit timeUnit = TimeUnit.valueOf(timer.getId().getBaseUnit()); addAttribute(COUNT, (new Double(timer.count())).longValue(), attributes); addAttribute(AVG, timer.mean(timeUnit), attributes); addAttribute(TOTAL_TIME, timer.totalTime(timeUnit), attributes); - addAttribute(TIME_UNIT, timeUnit.name().toLowerCase(), attributes); + addAttribute(TIME_UNIT, timeUnit.toString().toLowerCase(), attributes); //process meter's name, type and tags addMeterAsAttributes(timer.getId(), attributes); return attributes; diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java index a105631eed..d3e999459a 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java @@ -62,7 +62,6 @@ public class NewRelicHttpClientProviderImpl implements NewRelicClientProvider { private final HttpSender httpClient; private final String insightsEndpoint; private final NamingConvention namingConvention; - private TimeUnit timeUnit = TimeUnit.MILLISECONDS; @SuppressWarnings("deprecation") public NewRelicHttpClientProviderImpl(NewRelicConfig config) { @@ -92,9 +91,6 @@ public NewRelicHttpClientProviderImpl(NewRelicConfig config, HttpSender httpClie @Override public void publish(NewRelicMeterRegistry meterRegistry, List meters) { - - this.timeUnit = meterRegistry.getBaseTimeUnit(); - // New Relic's Insights API limits us to 1000 events per call // 1:1 mapping between Micrometer meters and New Relic events for (List batch : MeterPartition.partition(meterRegistry, Math.min(config.batchSize(), 1000))) { @@ -117,12 +113,13 @@ public void publish(NewRelicMeterRegistry meterRegistry, List meters) { } @Override - public Stream writeLongTaskTimer(LongTaskTimer ltt) { + public Stream writeLongTaskTimer(LongTaskTimer timer) { + TimeUnit timeUnit = TimeUnit.valueOf(timer.getId().getBaseUnit()); return Stream.of( - event(ltt.getId(), - new Attribute(ACTIVE_TASKS, ltt.activeTasks()), - new Attribute(DURATION, ltt.duration(timeUnit)), - new Attribute(TIME_UNIT, timeUnit.name().toLowerCase()) + event(timer.getId(), + new Attribute(ACTIVE_TASKS, timer.activeTasks()), + new Attribute(DURATION, timer.duration(timeUnit)), + new Attribute(TIME_UNIT, timeUnit.toString().toLowerCase()) ) ); } @@ -152,12 +149,12 @@ public Stream writeGauge(Gauge gauge) { @Override public Stream writeTimeGauge(TimeGauge gauge) { - Double value = gauge.value(timeUnit); + Double value = gauge.value(); if (Double.isFinite(value)) { return Stream.of( event(gauge.getId(), new Attribute(VALUE, value), - new Attribute(TIME_UNIT, timeUnit.name().toLowerCase()) + new Attribute(TIME_UNIT, gauge.baseTimeUnit().toString().toLowerCase()) ) ); } @@ -178,25 +175,27 @@ public Stream writeSummary(DistributionSummary summary) { @Override public Stream writeTimer(Timer timer) { + TimeUnit timeUnit = TimeUnit.valueOf(timer.getId().getBaseUnit()); return Stream.of( event(timer.getId(), new Attribute(COUNT, timer.count()), new Attribute(AVG, timer.mean(timeUnit)), new Attribute(TOTAL_TIME, timer.totalTime(timeUnit)), new Attribute(MAX, timer.max(timeUnit)), - new Attribute(TIME_UNIT, timeUnit.name().toLowerCase()) + new Attribute(TIME_UNIT, timeUnit.toString().toLowerCase()) ) ); } @Override public Stream writeFunctionTimer(FunctionTimer timer) { + TimeUnit timeUnit = TimeUnit.valueOf(timer.getId().getBaseUnit()); return Stream.of( event(timer.getId(), new Attribute(COUNT, timer.count()), new Attribute(AVG, timer.mean(timeUnit)), new Attribute(TOTAL_TIME, timer.totalTime(timeUnit)), - new Attribute(TIME_UNIT, timeUnit.name().toLowerCase()) + new Attribute(TIME_UNIT, timeUnit.toString().toLowerCase()) ) ); } diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java index 242937aba2..80122abed8 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java @@ -110,7 +110,7 @@ protected void publish() { @Override protected TimeUnit getBaseTimeUnit() { - return TimeUnit.MILLISECONDS; + return TimeUnit.SECONDS; } public static class Builder { diff --git a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java index 7111c4a1ee..1994ac45ef 100644 --- a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java +++ b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java @@ -131,7 +131,7 @@ void writeGauge() { Map expectedEntries = new HashMap<>(); expectedEntries.put("value", 1); writeGauge(meterNameEventTypeEnabledConfig, expectedEntries); - expectedEntries.put("metricName", "myGauge"); + expectedEntries.put("metricName", "myGauge2"); expectedEntries.put("metricType", "GAUGE"); writeGauge(agentConfig, expectedEntries); } @@ -143,8 +143,8 @@ private void writeGauge(NewRelicConfig config, String expectedJson) { } private void writeGauge(NewRelicConfig config, Map expectedEntries) { - registry.gauge("my.gauge", 1d); - Gauge gauge = registry.find("my.gauge").gauge(); + registry.gauge("my.gauge2", 1d); + Gauge gauge = registry.find("my.gauge2").gauge(); Map result = getAgentClientProvider(config).writeGauge(gauge); assertThat(result).hasSize(expectedEntries.size()); assertThat(result).containsExactlyEntriesOf(expectedEntries); @@ -169,8 +169,8 @@ private void writeGaugeShouldDropNanValue(NewRelicHttpClientProviderImpl clientP } private void writeGaugeShouldDropNanValue(NewRelicAgentClientProviderImpl clientProvider) { - registry.gauge("my.gauge", Double.NaN); - Gauge gauge = registry.find("my.gauge").gauge(); + registry.gauge("my.gauge2", Double.NaN); + Gauge gauge = registry.find("my.gauge2").gauge(); assertThat(clientProvider.writeGauge(gauge)).isEmpty(); } @@ -196,12 +196,12 @@ private void writeGaugeShouldDropInfiniteValues(NewRelicHttpClientProviderImpl c } private void writeGaugeShouldDropInfiniteValues(NewRelicAgentClientProviderImpl clientProvider) { - registry.gauge("my.gauge", Double.POSITIVE_INFINITY); - Gauge gauge = registry.find("my.gauge").gauge(); + registry.gauge("my.gauge2", Double.POSITIVE_INFINITY); + Gauge gauge = registry.find("my.gauge2").gauge(); assertThat(clientProvider.writeGauge(gauge)).isEmpty(); - registry.gauge("my.gauge", Double.NEGATIVE_INFINITY); - gauge = registry.find("my.gauge").gauge(); + registry.gauge("my.gauge2", Double.NEGATIVE_INFINITY); + gauge = registry.find("my.gauge2").gauge(); assertThat(clientProvider.writeGauge(gauge)).isEmpty(); } @@ -209,16 +209,16 @@ private void writeGaugeShouldDropInfiniteValues(NewRelicAgentClientProviderImpl void writeGaugeWithTimeGauge() { //test Http clientProvider writeGaugeWithTimeGauge(getHttpClientProvider(meterNameEventTypeEnabledConfig), - "{\"eventType\":\"myTimeGauge\",\"value\":1000,\"timeUnit\":\"milliseconds\"}"); + "{\"eventType\":\"myTimeGauge\",\"value\":1,\"timeUnit\":\"seconds\"}"); writeGaugeWithTimeGauge(getHttpClientProvider(httpConfig), - "{\"eventType\":\"MicrometerSample\",\"value\":1000,\"timeUnit\":\"milliseconds\",\"metricName\":\"myTimeGauge\",\"metricType\":\"GAUGE\"}"); + "{\"eventType\":\"MicrometerSample\",\"value\":1,\"timeUnit\":\"seconds\",\"metricName\":\"myTimeGauge\",\"metricType\":\"GAUGE\"}"); //test Agent clientProvider Map expectedEntries = new HashMap<>(); - expectedEntries.put("value", 1000); - expectedEntries.put("timeUnit", "milliseconds"); + expectedEntries.put("value", 1); + expectedEntries.put("timeUnit", "seconds"); writeGaugeWithTimeGauge(getAgentClientProvider(meterNameEventTypeEnabledConfig), expectedEntries); - expectedEntries.put("metricName", "myTimeGauge"); + expectedEntries.put("metricName", "myTimeGauge2"); expectedEntries.put("metricType", "GAUGE"); writeGaugeWithTimeGauge(getAgentClientProvider(agentConfig), expectedEntries); } @@ -232,8 +232,8 @@ private void writeGaugeWithTimeGauge(NewRelicHttpClientProviderImpl clientProvid private void writeGaugeWithTimeGauge(NewRelicAgentClientProviderImpl clientProvider, Map expectedEntries) { AtomicReference obj = new AtomicReference<>(1d); - registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); - TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); + registry.more().timeGauge("my.timeGauge2", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); + TimeGauge timeGauge = registry.find("my.timeGauge2").timeGauge(); Map result = clientProvider.writeTimeGauge(timeGauge); assertThat(result).hasSize(expectedEntries.size()); assertThat(result).containsExactlyEntriesOf(expectedEntries); @@ -259,8 +259,8 @@ private void writeGaugeWithTimeGaugeShouldDropNanValue(NewRelicHttpClientProvide private void writeGaugeWithTimeGaugeShouldDropNanValue(NewRelicAgentClientProviderImpl clientProvider) { AtomicReference obj = new AtomicReference<>(Double.NaN); - registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); - TimeGauge timeGauge = registry.find("my.timeGauge").timeGauge(); + registry.more().timeGauge("my.timeGauge2", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); + TimeGauge timeGauge = registry.find("my.timeGauge2").timeGauge(); assertThat(clientProvider.writeTimeGauge(timeGauge)).isEmpty(); } @@ -294,8 +294,8 @@ private void writeGaugeWithTimeGaugeShouldDropInfiniteValues(NewRelicAgentClient assertThat(clientProvider.writeTimeGauge(timeGauge)).isEmpty(); obj = new AtomicReference<>(Double.NEGATIVE_INFINITY); - registry.more().timeGauge("my.timeGauge", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); - timeGauge = registry.find("my.timeGauge").timeGauge(); + registry.more().timeGauge("my.timeGauge2", Tags.empty(), obj, TimeUnit.SECONDS, AtomicReference::get); + timeGauge = registry.find("my.timeGauge2").timeGauge(); assertThat(clientProvider.writeTimeGauge(timeGauge)).isEmpty(); } @@ -391,7 +391,7 @@ private void writeMeterWhenCustomMeterHasOnlyNonFiniteValuesShouldNotBeWritten( private void writeMeterWhenCustomMeterHasOnlyNonFiniteValuesShouldNotBeWritten( List measurements, NewRelicAgentClientProviderImpl clientProvider) { - Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(registry); + Meter meter = Meter.builder("my.meter2", Meter.Type.GAUGE, measurements).register(registry); assertThat(clientProvider.writeMeter(meter)).isEmpty(); } @@ -415,7 +415,7 @@ measurements, getHttpClientProvider(httpConfig), expectedEntries.put("value", 1); writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonFiniteValues( measurements, getAgentClientProvider(meterNameEventTypeEnabledConfig), expectedEntries); - expectedEntries.put("metricName", "myMeter"); + expectedEntries.put("metricName", "myMeter2"); expectedEntries.put("metricType", "GAUGE"); writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonFiniteValues( measurements, getAgentClientProvider(agentConfig), expectedEntries); @@ -429,7 +429,7 @@ private void writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkip private void writeMeterWhenCustomMeterHasMixedFiniteAndNonFiniteValuesShouldSkipOnlyNonFiniteValues( List measurements, NewRelicAgentClientProviderImpl clientProvider, Map expectedEntries) { - Meter meter = Meter.builder("my.meter", Meter.Type.GAUGE, measurements).register(registry); + Meter meter = Meter.builder("my.meter2", Meter.Type.GAUGE, measurements).register(registry); Map result = clientProvider.writeMeter(meter); assertThat(result).hasSize(expectedEntries.size()); assertThat(result).containsExactlyEntriesOf(expectedEntries); From e246fe137ec11aa3c180842be40c4dd8ffd3bee8 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Thu, 31 Oct 2019 17:21:15 -0500 Subject: [PATCH 14/25] make newrelic-api an optional dependency --- implementations/micrometer-registry-new-relic/build.gradle | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/implementations/micrometer-registry-new-relic/build.gradle b/implementations/micrometer-registry-new-relic/build.gradle index 9dd5203509..88e261b791 100644 --- a/implementations/micrometer-registry-new-relic/build.gradle +++ b/implementations/micrometer-registry-new-relic/build.gradle @@ -1,7 +1,9 @@ +apply plugin: 'nebula.optional-base' + dependencies { compile project(':micrometer-core') compile 'org.slf4j:slf4j-api:1.7.+' - compile 'com.newrelic.agent.java:newrelic-api:5.+' + compile 'com.newrelic.agent.java:newrelic-api:5.+', optional testCompile project(':micrometer-test') } \ No newline at end of file From fd8ced1b61b941e6bd37e565e05d0e226849fbac Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Tue, 17 Dec 2019 21:51:51 -0600 Subject: [PATCH 15/25] gitignore --- implementations/micrometer-registry-new-relic/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 implementations/micrometer-registry-new-relic/.gitignore diff --git a/implementations/micrometer-registry-new-relic/.gitignore b/implementations/micrometer-registry-new-relic/.gitignore new file mode 100644 index 0000000000..9bb88d3761 --- /dev/null +++ b/implementations/micrometer-registry-new-relic/.gitignore @@ -0,0 +1 @@ +/.DS_Store From 6412faf3b078e446f431850271766218083f11dc Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Tue, 17 Dec 2019 22:17:28 -0600 Subject: [PATCH 16/25] MeterRegistry updates from review --- .../NewRelicAgentClientProviderImpl.java | 5 ++- .../newrelic/NewRelicClientProvider.java | 4 +-- .../NewRelicHttpClientProviderImpl.java | 2 +- .../newrelic/NewRelicMeterRegistry.java | 18 ++-------- .../newrelic/NewRelicMeterRegistryTest.java | 33 +------------------ 5 files changed, 7 insertions(+), 55 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java index f880b93465..1467283f4a 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java @@ -16,7 +16,6 @@ package io.micrometer.newrelic; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -71,11 +70,11 @@ public NewRelicAgentClientProviderImpl(NewRelicConfig config, Agent newRelicAgen } @Override - public void publish(NewRelicMeterRegistry meterRegistry, List meters) { + public void publish(NewRelicMeterRegistry meterRegistry) { // New Relic's Java Agent Insights API is backed by a reservoir/buffer // and handles the actual publishing of events to New Relic. // 1:1 mapping between Micrometer meters and New Relic events - for (Meter meter : meters) { + for (Meter meter : meterRegistry.getMeters()) { sendEvents( meter.getId(), meter.match( diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java index b12185085e..1754c13873 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java @@ -15,8 +15,6 @@ */ package io.micrometer.newrelic; -import java.util.List; - import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.DistributionSummary; import io.micrometer.core.instrument.FunctionCounter; @@ -65,7 +63,7 @@ default String getEventType(Meter.Id id, NewRelicConfig config, NamingConvention return eventType; } - void publish(NewRelicMeterRegistry meterRegistry, List meters); + void publish(NewRelicMeterRegistry meterRegistry); Object writeFunctionTimer(FunctionTimer timer); diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java index d3e999459a..c92c47183c 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java @@ -90,7 +90,7 @@ public NewRelicHttpClientProviderImpl(NewRelicConfig config, HttpSender httpClie } @Override - public void publish(NewRelicMeterRegistry meterRegistry, List meters) { + public void publish(NewRelicMeterRegistry meterRegistry) { // New Relic's Insights API limits us to 1000 events per call // 1:1 mapping between Micrometer meters and New Relic events for (List batch : MeterPartition.partition(meterRegistry, Math.min(config.batchSize(), 1000))) { diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java index 80122abed8..f74848c0a6 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicMeterRegistry.java @@ -61,15 +61,7 @@ public NewRelicMeterRegistry(NewRelicConfig config, NewRelicClientProvider clien this(config, clientProvider, new NewRelicNamingConvention(), clock, DEFAULT_THREAD_FACTORY); } - /** - * @param config Configuration options for the registry that are describable as properties. - * @param clientProvider Provider of the HTTP or Agent-based client that publishes metrics to New Relic - * @param namingConvention Naming convention to apply before metric publishing - * @param clock The clock to use for timings. - * @param threadFactory The thread factory to use to create the publishing thread. - * @deprecated Use {@link #builder(NewRelicConfig)} instead. - */ - @Deprecated // VisibleForTesting + // VisibleForTesting NewRelicMeterRegistry(NewRelicConfig config, NewRelicClientProvider clientProvider, NamingConvention namingConvention, Clock clock, ThreadFactory threadFactory) { super(config, clock); @@ -77,12 +69,6 @@ public NewRelicMeterRegistry(NewRelicConfig config, NewRelicClientProvider clien if (clientProvider == null) { throw new MissingRequiredConfigurationException("clientProvider required to report metrics to New Relic"); } - if (namingConvention == null) { - throw new MissingRequiredConfigurationException("namingConvention must be set to report metrics to New Relic"); - } - if (threadFactory == null) { - throw new MissingRequiredConfigurationException("threadFactory must be set to report metrics to New Relic"); - } this.config = config; this.clientProvider = clientProvider; @@ -105,7 +91,7 @@ public void start(ThreadFactory threadFactory) { @Override protected void publish() { - clientProvider.publish(this, getMeters()); + clientProvider.publish(this); } @Override diff --git a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java index 1994ac45ef..1d64ba357f 100644 --- a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java +++ b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java @@ -50,7 +50,6 @@ import io.micrometer.core.instrument.TimeGauge; import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; -import io.micrometer.core.instrument.util.NamedThreadFactory; import io.micrometer.core.ipc.http.HttpSender; import io.micrometer.newrelic.NewRelicMeterRegistryTest.MockNewRelicAgent.MockNewRelicInsights; @@ -575,36 +574,6 @@ public String get(String key) { .hasMessageContaining("clientProvider"); } - @SuppressWarnings("deprecation") - @Test - void failsConfigMissingNamingConvention() { - NewRelicConfig config = new NewRelicConfig() { - @Override - public String get(String key) { - return null; - } - }; - - assertThatThrownBy(() -> new NewRelicMeterRegistry(config, new MockClientProvider(), null, clock, new NamedThreadFactory("test"))) - .isExactlyInstanceOf(MissingRequiredConfigurationException.class) - .hasMessageContaining("namingConvention"); - } - - @SuppressWarnings("deprecation") - @Test - void failsConfigMissingThreadFactory() { - NewRelicConfig config = new NewRelicConfig() { - @Override - public String get(String key) { - return null; - } - }; - - assertThatThrownBy(() -> new NewRelicMeterRegistry(config, new MockClientProvider(), new NewRelicNamingConvention(), clock, null)) - .isExactlyInstanceOf(MissingRequiredConfigurationException.class) - .hasMessageContaining("threadFactory"); - } - @Test void failsConfigHttpMissingEventType() { NewRelicConfig config = new NewRelicConfig() { @@ -785,7 +754,7 @@ public Request getRequest() { class MockClientProvider implements NewRelicClientProvider { @Override - public void publish(NewRelicMeterRegistry meterRegistry, List meters) { + public void publish(NewRelicMeterRegistry meterRegistry) { //No-op } From e9a844e341b8fe674d23e104f2e0f623de01ee96 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Tue, 17 Dec 2019 22:23:46 -0600 Subject: [PATCH 17/25] remove redundant modifiers --- .../newrelic/NewRelicClientProvider.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java index 1754c13873..02635ebd93 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicClientProvider.java @@ -31,25 +31,25 @@ */ public interface NewRelicClientProvider { //long task timer - static final String DURATION = "duration"; - static final String ACTIVE_TASKS = "activeTasks"; + String DURATION = "duration"; + String ACTIVE_TASKS = "activeTasks"; //distribution summary & timer - static final String MAX = "max"; - static final String TOTAL = "total"; - static final String AVG = "avg"; - static final String COUNT = "count"; + String MAX = "max"; + String TOTAL = "total"; + String AVG = "avg"; + String COUNT = "count"; //timer - static final String TOTAL_TIME = "totalTime"; - static final String TIME = "time"; + String TOTAL_TIME = "totalTime"; + String TIME = "time"; //gauge - static final String VALUE = "value"; + String VALUE = "value"; //counter - static final String THROUGHPUT = "throughput"; //TODO Why not "count"? ..confusing if just counting something + String THROUGHPUT = "throughput"; //TODO Why not "count"? ..confusing if just counting something //timer - static final String TIME_UNIT = "timeUnit"; + String TIME_UNIT = "timeUnit"; //all - static final String METRIC_TYPE = "metricType"; - static final String METRIC_NAME = "metricName"; + String METRIC_TYPE = "metricType"; + String METRIC_NAME = "metricName"; default String getEventType(Meter.Id id, NewRelicConfig config, NamingConvention namingConvention) { String eventType = null; From 3f89a097b7ad81e7f512efbfdc509ea8d9f2bf87 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Tue, 17 Dec 2019 22:40:04 -0600 Subject: [PATCH 18/25] specific arg type in sendEvents --- .../NewRelicAgentClientProviderImpl.java | 21 ++++++-------- .../NewRelicHttpClientProviderImpl.java | 28 ++++++++----------- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java index 1467283f4a..a7f55454b6 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicAgentClientProviderImpl.java @@ -237,19 +237,14 @@ void addAttribute(String key, String value, Map attributes) { attributes.put(namingConvention.tagKey(key), namingConvention.tagValue(value)); } - void sendEvents(Meter.Id id, Object attributesObj) { - if (attributesObj instanceof Map) { - @SuppressWarnings("unchecked") - Map attributes = (Map)attributesObj; - - //Delegate to New Relic Java Agent - if (attributes != null && attributes.isEmpty() == false) { - String eventType = getEventType(id, config, namingConvention); - try { - newRelicAgent.getInsights().recordCustomEvent(eventType, attributes); - } catch (Throwable e) { - logger.warn("failed to send metrics to new relic", e); - } + void sendEvents(Meter.Id id, Map attributes) { + //Delegate to New Relic Java Agent + if (attributes != null && attributes.isEmpty() == false) { + String eventType = getEventType(id, config, namingConvention); + try { + newRelicAgent.getInsights().recordCustomEvent(eventType, attributes); + } catch (Throwable e) { + logger.warn("failed to send metrics to new relic", e); } } } diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java index c92c47183c..9b1003d803 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java @@ -257,22 +257,18 @@ private String event(Meter.Id id, Iterable extraTags, Attribute... attribut .collect(Collectors.joining("", "{\"eventType\":\"" + escapeJson(eventType) + "\"", tagsJson + "}")); } - void sendEvents(Meter.Id id, Object attributesObj) { - if (attributesObj instanceof Stream) { - @SuppressWarnings("unchecked") - Stream events = (Stream)attributesObj; - try { - AtomicInteger totalEvents = new AtomicInteger(); - - httpClient.post(insightsEndpoint) - .withHeader("X-Insert-Key", config.apiKey()) - .withJsonContent(events.peek(ev -> totalEvents.incrementAndGet()).collect(Collectors.joining(",", "[", "]"))) - .send() - .onSuccess(response -> logger.debug("successfully sent {} metrics to New Relic.", totalEvents)) - .onError(response -> logger.error("failed to send metrics to new relic: http {} {}", response.code(), response.body())); - } catch (Throwable e) { - logger.warn("failed to send metrics to new relic", e); - } + void sendEvents(Meter.Id id, Stream events) { + try { + AtomicInteger totalEvents = new AtomicInteger(); + + httpClient.post(insightsEndpoint) + .withHeader("X-Insert-Key", config.apiKey()) + .withJsonContent(events.peek(ev -> totalEvents.incrementAndGet()).collect(Collectors.joining(",", "[", "]"))) + .send() + .onSuccess(response -> logger.debug("successfully sent {} metrics to New Relic.", totalEvents)) + .onError(response -> logger.error("failed to send metrics to new relic: http {} {}", response.code(), response.body())); + } catch (Throwable e) { + logger.warn("failed to send metrics to new relic", e); } } From 6a5712f6542c68ebe5d1e49f16b33bc35a61d5fe Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Tue, 17 Dec 2019 22:58:13 -0600 Subject: [PATCH 19/25] reverted publish to using stream().flatMap for processing batches --- .../NewRelicHttpClientProviderImpl.java | 27 ++++++++----------- .../newrelic/NewRelicMeterRegistryTest.java | 4 +-- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java index 9b1003d803..25dcff1f24 100644 --- a/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java +++ b/implementations/micrometer-registry-new-relic/src/main/java/io/micrometer/newrelic/NewRelicHttpClientProviderImpl.java @@ -94,21 +94,16 @@ public void publish(NewRelicMeterRegistry meterRegistry) { // New Relic's Insights API limits us to 1000 events per call // 1:1 mapping between Micrometer meters and New Relic events for (List batch : MeterPartition.partition(meterRegistry, Math.min(config.batchSize(), 1000))) { - for (Meter meter : batch) { - sendEvents(meter.getId(), - meter.match( - this::writeGauge, - this::writeCounter, - this::writeTimer, - this::writeSummary, - this::writeLongTaskTimer, - this::writeTimeGauge, - this::writeFunctionCounter, - this::writeFunctionTimer, - this::writeMeter - ) - ); - } + sendEvents(batch.stream().flatMap(meter -> meter.match( + this::writeGauge, + this::writeCounter, + this::writeTimer, + this::writeSummary, + this::writeLongTaskTimer, + this::writeTimeGauge, + this::writeFunctionCounter, + this::writeFunctionTimer, + this::writeMeter))); } } @@ -257,7 +252,7 @@ private String event(Meter.Id id, Iterable extraTags, Attribute... attribut .collect(Collectors.joining("", "{\"eventType\":\"" + escapeJson(eventType) + "\"", tagsJson + "}")); } - void sendEvents(Meter.Id id, Stream events) { + void sendEvents(Stream events) { try { AtomicInteger totalEvents = new AtomicInteger(); diff --git a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java index 1d64ba357f..0907bdafe0 100644 --- a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java +++ b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java @@ -469,7 +469,7 @@ void sendEventsWithHttpProvider() { registry.gauge("my.gauge", 1d); Gauge gauge = registry.find("my.gauge").gauge(); - httpProvider.sendEvents(gauge.getId(), httpProvider.writeGauge(gauge)); + httpProvider.sendEvents(httpProvider.writeGauge(gauge)); assertThat(new String(mockHttpClient.getRequest().getEntity())) .contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\"}"); @@ -482,7 +482,7 @@ void sendEventsWithHttpProvider() { registry.gauge("my.gauge2", 1d); gauge = registry.find("my.gauge2").gauge(); - httpProvider.sendEvents(gauge.getId(), httpProvider.writeGauge(gauge)); + httpProvider.sendEvents(httpProvider.writeGauge(gauge)); assertThat(new String(mockHttpClient.getRequest().getEntity())) .contains("{\"eventType\":\"myGauge2\",\"value\":1}"); From aee20cd962a5c6819ab33020bee7602239d0e15b Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Tue, 17 Dec 2019 23:53:17 -0600 Subject: [PATCH 20/25] improve http client provider publish test to ensure batching --- .../io/micrometer/newrelic/NewRelicMeterRegistryTest.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java index 0907bdafe0..a10b22c78b 100644 --- a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java +++ b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java @@ -534,10 +534,16 @@ void publishWithHttpClientProvider() { Gauge gauge = registry.find("my.gauge").gauge(); assertThat(gauge).isNotNull(); + registry.gauge("other.gauge", 2d); + Gauge other = registry.find("other.gauge").gauge(); + assertThat(other).isNotNull(); + registry.publish(); + //should send a batch of multiple assertThat(new String(mockHttpClient.getRequest().getEntity())) - .contains("{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\"}"); + .contains("[{\"eventType\":\"MicrometerSample\",\"value\":2,\"metricName\":\"otherGauge\",\"metricType\":\"GAUGE\"}," + + "{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\"}]"); } @Test From 5fc6f668e3aef0e620ed615c7c06f9acc0c526c6 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Wed, 18 Dec 2019 06:28:14 -0600 Subject: [PATCH 21/25] improve http and agent publishing tests --- .../newrelic/NewRelicMeterRegistryTest.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java index a10b22c78b..bd73ffcbd4 100644 --- a/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java +++ b/implementations/micrometer-registry-new-relic/src/test/java/io/micrometer/newrelic/NewRelicMeterRegistryTest.java @@ -530,7 +530,7 @@ void publishWithHttpClientProvider() { NewRelicMeterRegistry registry = new NewRelicMeterRegistry(httpConfig, httpProvider, clock); - registry.gauge("my.gauge", 1d); + registry.gauge("my.gauge", Tags.of("theTag", "theValue"), 1d); Gauge gauge = registry.find("my.gauge").gauge(); assertThat(gauge).isNotNull(); @@ -540,10 +540,10 @@ void publishWithHttpClientProvider() { registry.publish(); - //should send a batch of multiple + //should send a batch of multiple in one json payload assertThat(new String(mockHttpClient.getRequest().getEntity())) .contains("[{\"eventType\":\"MicrometerSample\",\"value\":2,\"metricName\":\"otherGauge\",\"metricType\":\"GAUGE\"}," + - "{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\"}]"); + "{\"eventType\":\"MicrometerSample\",\"value\":1,\"metricName\":\"myGauge\",\"metricType\":\"GAUGE\",\"theTag\":\"theValue\"}]"); } @Test @@ -555,15 +555,20 @@ void publishWithAgentClientProvider() { NewRelicMeterRegistry registry = new NewRelicMeterRegistry(agentConfig, agentProvider, clock); - registry.gauge("my.gauge", 1d); + registry.gauge("my.gauge", Tags.of("theTag", "theValue"), 1d); Gauge gauge = registry.find("my.gauge").gauge(); - assertThat(gauge).isNotNull(); + assertThat(gauge).isNotNull(); + + registry.gauge("other.gauge", 2d); + Gauge other = registry.find("other.gauge").gauge(); + assertThat(other).isNotNull(); registry.publish(); + //should delegate to the Agent one at a time assertThat(((MockNewRelicInsights)mockNewRelicAgent.getInsights()).getInsightData().getEventType()).isEqualTo("MicrometerSample"); Map result = ((MockNewRelicInsights)mockNewRelicAgent.getInsights()).getInsightData().getAttributes(); - assertThat(result).hasSize(3); + assertThat(result).hasSize(4); } @Test From 77ff2a92fafc62c848ded06fe8fc7915299cb678 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Wed, 11 Mar 2020 21:34:14 -0500 Subject: [PATCH 22/25] removed optional --- implementations/micrometer-registry-new-relic/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/implementations/micrometer-registry-new-relic/build.gradle b/implementations/micrometer-registry-new-relic/build.gradle index 292b754f98..2977c51d18 100644 --- a/implementations/micrometer-registry-new-relic/build.gradle +++ b/implementations/micrometer-registry-new-relic/build.gradle @@ -2,7 +2,7 @@ dependencies { api project(':micrometer-core') implementation 'org.slf4j:slf4j-api' - implementation 'com.newrelic.agent.java:newrelic-api:5.+', optional + implementation 'com.newrelic.agent.java:newrelic-api:5.+' testImplementation project(':micrometer-test') } \ No newline at end of file From 6292403542ae55fdf2cf957ef5dc2a44af3c69c9 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Wed, 11 Mar 2020 22:06:01 -0500 Subject: [PATCH 23/25] switch newrelic-api to compileOnly --- implementations/micrometer-registry-new-relic/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/implementations/micrometer-registry-new-relic/build.gradle b/implementations/micrometer-registry-new-relic/build.gradle index 2977c51d18..31279ded2d 100644 --- a/implementations/micrometer-registry-new-relic/build.gradle +++ b/implementations/micrometer-registry-new-relic/build.gradle @@ -2,7 +2,7 @@ dependencies { api project(':micrometer-core') implementation 'org.slf4j:slf4j-api' - implementation 'com.newrelic.agent.java:newrelic-api:5.+' + compileOnly 'com.newrelic.agent.java:newrelic-api:5.+' testImplementation project(':micrometer-test') } \ No newline at end of file From b1437d07c217d0140ad02a2b726b8369f89838c4 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Wed, 11 Mar 2020 22:21:52 -0500 Subject: [PATCH 24/25] use optionalApi for newrelic-api --- implementations/micrometer-registry-new-relic/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/implementations/micrometer-registry-new-relic/build.gradle b/implementations/micrometer-registry-new-relic/build.gradle index 31279ded2d..633852ab60 100644 --- a/implementations/micrometer-registry-new-relic/build.gradle +++ b/implementations/micrometer-registry-new-relic/build.gradle @@ -2,7 +2,7 @@ dependencies { api project(':micrometer-core') implementation 'org.slf4j:slf4j-api' - compileOnly 'com.newrelic.agent.java:newrelic-api:5.+' + optionalApi 'com.newrelic.agent.java:newrelic-api:5.+' testImplementation project(':micrometer-test') } \ No newline at end of file From 12ea291e135ee12ba5ad1ee8155e912b40a39223 Mon Sep 17 00:00:00 2001 From: Neil Powell <52715665+neiljpowell@users.noreply.github.com> Date: Wed, 11 Mar 2020 22:24:45 -0500 Subject: [PATCH 25/25] implementation newrelic-api --- implementations/micrometer-registry-new-relic/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/implementations/micrometer-registry-new-relic/build.gradle b/implementations/micrometer-registry-new-relic/build.gradle index 633852ab60..2977c51d18 100644 --- a/implementations/micrometer-registry-new-relic/build.gradle +++ b/implementations/micrometer-registry-new-relic/build.gradle @@ -2,7 +2,7 @@ dependencies { api project(':micrometer-core') implementation 'org.slf4j:slf4j-api' - optionalApi 'com.newrelic.agent.java:newrelic-api:5.+' + implementation 'com.newrelic.agent.java:newrelic-api:5.+' testImplementation project(':micrometer-test') } \ No newline at end of file