Conversation
| @Override | ||
| public JsonElement serialize( | ||
| HistogramBucket src, Type typeOfSrc, JsonSerializationContext context) { | ||
| JsonObject jsonObject = new JsonObject(); | ||
| if (Double.isInfinite(src.getLe()) && src.getLe() > 0) { | ||
| jsonObject.addProperty("le", "+Inf"); | ||
| } else { | ||
| jsonObject.addProperty("le", src.getLe()); | ||
| } | ||
| jsonObject.addProperty("count", src.getCount()); | ||
| return jsonObject; | ||
| } | ||
| } |
There was a problem hiding this comment.
We are adding simple json serializer for Double.POSITIVE_INFINITY.
| .registerTypeAdapter(LocalDateTime.class, new DateTimeSerializer()) | ||
| .registerTypeAdapter(Instant.class, new InstantSerializer()) | ||
| .registerTypeAdapter(AtomicLong.class, new AtomicLongSerializer()) | ||
| .registerTypeAdapter(HistogramBucket.class, new HistogramBucketSerializer()) |
There was a problem hiding this comment.
Register our serializer
| new GsonBuilder() | ||
| .registerTypeAdapter(LocalDateTime.class, new DateTimeSerializer()) | ||
| .registerTypeAdapter(AtomicLong.class, new AtomicLongSerializer()) | ||
| .registerTypeAdapter(HistogramBucket.class, new HistogramBucketSerializer()) |
There was a problem hiding this comment.
Register serializer.
Pull Request Test Coverage Report for Build 20329778924Details
💛 - Coveralls |
| ClientMetrics( | ||
| UnleashConfig config, | ||
| @Nullable MetricsBucket bucket, | ||
| @Nullable List<CollectedMetric> impactMetrics) { |
There was a problem hiding this comment.
Adding impact metrics so it can be used in next iterations.
| for (Double le : buckets) { | ||
| bucketSamples.add( | ||
| new HistogramBucket(le, data.buckets.getOrDefault(le, 0L))); | ||
| bucketSamples.add(new HistogramBucket(le, data.buckets.getOrDefault(le, 0L))); |
| @Nullable Authenticator proxyAuthenticator, | ||
| @Nullable Consumer<UnleashException> startupExceptionHandler) { | ||
| @Nullable Consumer<UnleashException> startupExceptionHandler, | ||
| @Nullable ImpactMetricsDataSource impactMetricsRegistry) { |
There was a problem hiding this comment.
In Java it is possible to build config by adding custom registry.
| import io.getunleash.impactmetrics.HistogramBucket; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class HistogramBucketSerializerTest { |
There was a problem hiding this comment.
Adding basic tests to validate our serializer works.
There was a problem hiding this comment.
can't we test this in metrics tests or repo tests as you called it? it will be closer to the Node testing and also you will test the entire behavior including serialization.
There was a problem hiding this comment.
another option is to have one broader test that will verify that we registered our serializer and when someone removed the registration accidentally tests will catch it
There was a problem hiding this comment.
Yes, I can theoretically add it when I get to HTTP layer.
There was a problem hiding this comment.
Currently moved to metric type test
sjaanus
changed the title
4 participants
Histogram serialization and preparing to send with metrics. Doing the wiring.