From c164ffd21bd52b6c4b9b017ec350c7eb0b1cbaed Mon Sep 17 00:00:00 2001 From: Drew Michel Date: Wed, 21 Nov 2018 14:01:34 -0500 Subject: [PATCH 1/5] [pubsub] ignore events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit don’t sent events into the metrics pipeline --- .../com/spotify/ffwd/pubsub/PubsubPluginSink.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/modules/pubsub/src/main/java/com/spotify/ffwd/pubsub/PubsubPluginSink.java b/modules/pubsub/src/main/java/com/spotify/ffwd/pubsub/PubsubPluginSink.java index 97a0db05..9feec884 100644 --- a/modules/pubsub/src/main/java/com/spotify/ffwd/pubsub/PubsubPluginSink.java +++ b/modules/pubsub/src/main/java/com/spotify/ffwd/pubsub/PubsubPluginSink.java @@ -90,16 +90,7 @@ public void init() { } @Override public AsyncFuture sendEvents(Collection events) { - for (Event event : events) { - try { - publisher.publish(PubsubMessage.newBuilder() - .setData(ByteString.copyFrom(serializer.serialize(event))).build() - ); - } catch (Exception e) { - log.error("Failed to publish event {}", e); - } - } - + log.debug("Sending events is not supported!"); return async.resolved(); } From 3b98544a78534ba050af611f6bc6544d4be8a0b3 Mon Sep 17 00:00:00 2001 From: Drew Michel Date: Wed, 21 Nov 2018 14:04:15 -0500 Subject: [PATCH 2/5] [core] add protobuf serializer Add a protobuf serializer that is more space and cpu efficient. --- core/pom.xml | 5 ++ .../ffwd/serializer/BuiltInSerializers.java | 1 + .../serializer/Spotify100ProtoSerializer.java | 48 +++++++++++++ core/src/main/proto/spotify_100.proto | 27 +++++++ .../TestSpotify100ProtoSerializer.java | 71 +++++++++++++++++++ pom.xml | 30 ++++++++ 6 files changed, 182 insertions(+) create mode 100644 core/src/main/java/com/spotify/ffwd/serializer/Spotify100ProtoSerializer.java create mode 100644 core/src/main/proto/spotify_100.proto create mode 100644 core/src/test/java/com/spotify/ffwd/serializer/TestSpotify100ProtoSerializer.java diff --git a/core/pom.xml b/core/pom.xml index 67ae0926..474f4baa 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -25,6 +25,11 @@ provided + + com.google.protobuf + protobuf-java + + eu.toolchain.async tiny-async-core diff --git a/core/src/main/java/com/spotify/ffwd/serializer/BuiltInSerializers.java b/core/src/main/java/com/spotify/ffwd/serializer/BuiltInSerializers.java index 63a793b8..16c964e1 100644 --- a/core/src/main/java/com/spotify/ffwd/serializer/BuiltInSerializers.java +++ b/core/src/main/java/com/spotify/ffwd/serializer/BuiltInSerializers.java @@ -31,6 +31,7 @@ public class BuiltInSerializers implements FastForwardModule { @Override public void setup() throws Exception { context.registerSerializer("spotify100", Spotify100Serializer.class); + context.registerSerializer("spotify100proto", Spotify100ProtoSerializer.class); context.registerSerializer("to-string", ToStringSerializer.class); } } diff --git a/core/src/main/java/com/spotify/ffwd/serializer/Spotify100ProtoSerializer.java b/core/src/main/java/com/spotify/ffwd/serializer/Spotify100ProtoSerializer.java new file mode 100644 index 00000000..a32f92e9 --- /dev/null +++ b/core/src/main/java/com/spotify/ffwd/serializer/Spotify100ProtoSerializer.java @@ -0,0 +1,48 @@ +/*- + * -\-\- + * FastForward Core + * -- + * Copyright (C) 2016 - 2018 Spotify AB + * -- + * 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 + * + * http://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 com.spotify.ffwd.serializer; + +import com.spotify.ffwd.model.Event; +import com.spotify.ffwd.model.Metric; +import com.spotify.proto.Spotify100; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class Spotify100ProtoSerializer implements Serializer { + + @Override + public byte[] serialize(final Event event) throws Exception { + log.debug("Serializing events is not supported"); + return new byte[0]; + } + + @Override + public byte[] serialize(final Metric metric) throws Exception { + return Spotify100.Metric.newBuilder() + .setKey(metric.getKey()) + .setTime(metric.getTime().getTime()) + .setValue(metric.getValue()) + .putAllTags(metric.getTags()) + .putAllResource(metric.getResource()) + .build() + .toByteArray(); + } +} diff --git a/core/src/main/proto/spotify_100.proto b/core/src/main/proto/spotify_100.proto new file mode 100644 index 00000000..bf17c995 --- /dev/null +++ b/core/src/main/proto/spotify_100.proto @@ -0,0 +1,27 @@ +syntax = "proto3"; + +package spotify.metric; +option java_package = "com.spotify.proto"; + +option optimize_for = SPEED; + +message Metric { + // key of metric. + string key = 1; + + // time in ms when metric was generated. + int64 time = 2; + + // value of metric. + double value = 3; + + // tags associated to a metric, used to be referred to as attributes. + map tags = 4; + + // resource "tags" associated to metric. + map resource = 5; +} + +message Message { + Metric metric = 1; +} diff --git a/core/src/test/java/com/spotify/ffwd/serializer/TestSpotify100ProtoSerializer.java b/core/src/test/java/com/spotify/ffwd/serializer/TestSpotify100ProtoSerializer.java new file mode 100644 index 00000000..9bd176fc --- /dev/null +++ b/core/src/test/java/com/spotify/ffwd/serializer/TestSpotify100ProtoSerializer.java @@ -0,0 +1,71 @@ +/*- + * -\-\- + * FastForward Core + * -- + * Copyright (C) 2016 - 2018 Spotify AB + * -- + * 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 + * + * http://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 com.spotify.ffwd.serializer; + +import static org.hamcrest.CoreMatchers.any; +import static org.junit.Assert.assertThat; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.spotify.ffwd.model.Metric; +import java.time.Instant; +import java.util.Date; +import org.junit.Before; +import org.junit.Test; + +public class TestSpotify100ProtoSerializer { + private Spotify100ProtoSerializer spotify100ProtoSerializer; + private Metric metric; + + @Before + public void setup() { + spotify100ProtoSerializer = new Spotify100ProtoSerializer(); + } + + @Test + public void testSerializeMetric() throws Exception { + metric = new Metric("", + 0.0, + Date.from(Instant.ofEpochSecond(1542812184)), + ImmutableSet.of(), + ImmutableMap.of("a", "b"), + ImmutableMap.of(), + ""); + + final byte[] serialize = spotify100ProtoSerializer.serialize(metric); + + assertThat(serialize, any(byte[].class)); + } + + @Test(expected = NullPointerException.class) + public void testNullKeyException() throws Exception { + metric = new Metric(null, + 0.0, + Date.from(Instant.ofEpochSecond(1542812184)), + ImmutableSet.of(), + ImmutableMap.of("a", "b"), + ImmutableMap.of(), + ""); + + spotify100ProtoSerializer.serialize(metric); + } + +} diff --git a/pom.xml b/pom.xml index 84ef3dec..26f212ab 100644 --- a/pom.xml +++ b/pom.xml @@ -224,6 +224,12 @@ ${project.version} + + com.google.protobuf + protobuf-java + 3.5.1 + + com.fasterxml.jackson.core jackson-databind @@ -336,7 +342,31 @@ + + + kr.motd.maven + os-maven-plugin + 1.5.0.Final + + + + + org.xolstice.maven.plugins + protobuf-maven-plugin + 0.6.1 + + com.google.protobuf:protoc:3.6.1:exe:${os.detected.classifier} + + + + + compile + + + + + org.codehaus.mojo findbugs-maven-plugin From 675bb3506a8d08f8e4d2a340b084019c40492a7c Mon Sep 17 00:00:00 2001 From: Drew Michel Date: Sun, 25 Nov 2018 09:50:20 -0500 Subject: [PATCH 3/5] [json] regression test for null value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A JSON null value is being translated to a “null” string. Was supposed to be fixed in https://github.com/FasterXML/jackson-databind/issues/1842 but it doesn’t appear to be. --- modules/json/pom.xml | 8 +++++ .../ffwd/json/JsonObjectMapperDecoder.java | 2 +- .../json/JsonObjectMapperDecoderTest.java | 29 +++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 modules/json/src/test/java/com/spotify/ffwd/json/JsonObjectMapperDecoderTest.java diff --git a/modules/json/pom.xml b/modules/json/pom.xml index 9f268b29..435ed97a 100644 --- a/modules/json/pom.xml +++ b/modules/json/pom.xml @@ -23,5 +23,13 @@ com.spotify.ffwd ffwd-api + + + + junit + junit + test + + diff --git a/modules/json/src/main/java/com/spotify/ffwd/json/JsonObjectMapperDecoder.java b/modules/json/src/main/java/com/spotify/ffwd/json/JsonObjectMapperDecoder.java index 6c78a656..547c9985 100644 --- a/modules/json/src/main/java/com/spotify/ffwd/json/JsonObjectMapperDecoder.java +++ b/modules/json/src/main/java/com/spotify/ffwd/json/JsonObjectMapperDecoder.java @@ -169,7 +169,7 @@ private double decodeDouble(JsonNode tree, String name) { return n.asDouble(); } - private String decodeString(JsonNode tree, String name) { + String decodeString(JsonNode tree, String name) { final JsonNode n = tree.get(name); if (n == null) { diff --git a/modules/json/src/test/java/com/spotify/ffwd/json/JsonObjectMapperDecoderTest.java b/modules/json/src/test/java/com/spotify/ffwd/json/JsonObjectMapperDecoderTest.java new file mode 100644 index 00000000..a3d52938 --- /dev/null +++ b/modules/json/src/test/java/com/spotify/ffwd/json/JsonObjectMapperDecoderTest.java @@ -0,0 +1,29 @@ +package com.spotify.ffwd.json; + +import static junit.framework.TestCase.assertNull; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; +import org.junit.Before; +import org.junit.Test; + +public class JsonObjectMapperDecoderTest { + private final ObjectMapper mapper = new ObjectMapper(); + + private JsonObjectMapperDecoder j; + + @Before + public void setup() { + j = new JsonObjectMapperDecoder(); + } + + @Test + public void nullKey() throws IOException { + final JsonNode json = mapper.readTree("{\"key\": null}"); + final String key = j.decodeString(json, "key"); + + assertNull(key); + } + +} From 2be52102771c8727d5ea970aac9b657467280fd6 Mon Sep 17 00:00:00 2001 From: Drew Michel Date: Sun, 25 Nov 2018 09:51:37 -0500 Subject: [PATCH 4/5] [json] fix bug with null value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A json field with a null value was being serialized to a “null” string. --- .../ffwd/json/JsonObjectMapperDecoder.java | 2 +- .../json/JsonObjectMapperDecoderTest.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/modules/json/src/main/java/com/spotify/ffwd/json/JsonObjectMapperDecoder.java b/modules/json/src/main/java/com/spotify/ffwd/json/JsonObjectMapperDecoder.java index 547c9985..a846dca0 100644 --- a/modules/json/src/main/java/com/spotify/ffwd/json/JsonObjectMapperDecoder.java +++ b/modules/json/src/main/java/com/spotify/ffwd/json/JsonObjectMapperDecoder.java @@ -172,7 +172,7 @@ private double decodeDouble(JsonNode tree, String name) { String decodeString(JsonNode tree, String name) { final JsonNode n = tree.get(name); - if (n == null) { + if (n.isNull()) { return null; } diff --git a/modules/json/src/test/java/com/spotify/ffwd/json/JsonObjectMapperDecoderTest.java b/modules/json/src/test/java/com/spotify/ffwd/json/JsonObjectMapperDecoderTest.java index a3d52938..07360aa8 100644 --- a/modules/json/src/test/java/com/spotify/ffwd/json/JsonObjectMapperDecoderTest.java +++ b/modules/json/src/test/java/com/spotify/ffwd/json/JsonObjectMapperDecoderTest.java @@ -1,3 +1,23 @@ +/*- + * -\-\- + * FastForward JSON Module + * -- + * Copyright (C) 2016 - 2018 Spotify AB + * -- + * 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 + * + * http://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 com.spotify.ffwd.json; import static junit.framework.TestCase.assertNull; From 0ba46688aeb42a7514a09ffdb8084ed5229aac2b Mon Sep 17 00:00:00 2001 From: Drew Michel Date: Sun, 25 Nov 2018 09:52:17 -0500 Subject: [PATCH 5/5] Bump semantic-metrics to latest version Which has support support for Java 11. --- agent/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/pom.xml b/agent/pom.xml index fa689f31..bfab529d 100644 --- a/agent/pom.xml +++ b/agent/pom.xml @@ -14,7 +14,7 @@ - 0.7.0 + 1.0.1