Changes to generate suitable load in otlp and statsd metric #874
Conversation
...nerator/src/main/java/com/amazon/opentelemetry/load/generator/emitter/OtlpMetricEmitter.java
Outdated
Show resolved
Hide resolved
| case "gauge": | ||
| for(int i=0 ; i < param.getMetricCount(); i++) { | ||
| for (int id = 0; id < param.getDatapointCount(); id++) { | ||
| this.gaugeValues.add(id, ThreadLocalRandom.current().nextLong(-100, 100)); |
There was a problem hiding this comment.
I think this will have unintended effects no? https://docs.oracle.com/javase/8/docs/api/java/util/ArrayList.html#add-int-E-
Inserts the specified element at the specified position in this list. Shifts the element currently at that position (if any) and any subsequent elements to the right (adds one to their indices).
Every time that you call nextDatapoint, more data will be added to the array. if this gets called enough, you might become out of memory.
maybe use plain array new long[param.getDataPointCount]?
| switch (param.getMetricType()) { | ||
| case "counter": | ||
| for(LongCounter counter: counters) { | ||
| for(int id=0 ; id < param.getDatapointCount(); id++) { | ||
| Attributes datapointAttributes = getDataPointAttributes(id); | ||
| counter.add(ThreadLocalRandom.current().nextInt(1,10), datapointAttributes); | ||
| } | ||
| } | ||
| break; | ||
| case "gauge": | ||
| for(int i=0 ; i < param.getMetricCount(); i++) { | ||
| for (int id = 0; id < param.getDatapointCount(); id++) { | ||
| this.gaugeValues.add(id, ThreadLocalRandom.current().nextLong(-100, 100)); | ||
| } | ||
| } | ||
| break; |
There was a problem hiding this comment.
nit: I think you could use a Template class pattern or a strategy pattern in order to make the code cleaner instead of having to deal with these two different cases. The idea is that you instantiate a different implementation based on the metric type.
There was a problem hiding this comment.
I have not updated code with this change, as I thought it is just a simple switch case in sample-app. But do let me know if it has to be done, then I will make the necessary changes.
| case "counter": | ||
| for(LongCounter counter: counters) { | ||
| for(int id=0 ; id < param.getDatapointCount(); id++) { | ||
| Attributes datapointAttributes = getDataPointAttributes(id); |
There was a problem hiding this comment.
Can you attach attributes to counters at creation time or does it have to be done here?
There was a problem hiding this comment.
as each datapoint has a differentiating(at least one distinct) label, so I think it has to be done here (at the time of datapoint creation).
| @@ -77,22 +104,52 @@ public void setupProvider() { | |||
| GlobalOpenTelemetry.meterBuilder("aws-otel-load-generator-metric") | |||
There was a problem hiding this comment.
| GlobalOpenTelemetry.meterBuilder("aws-otel-load-generator-metric") | |
| GlobalOpenTelemetry.meterBuilder("adot-load-generator-metric") |
| .append("|g|").append(attributes).append(id).append("\n"); | ||
| } | ||
| } | ||
| buf = payload.toString().getBytes(); | ||
| DatagramPacket packet = new DatagramPacket(buf, buf.length, ipAddress, portAddress); |
There was a problem hiding this comment.
Just fyi, udp packets have size limitation of 64k. I think it is ok to let the exception bubble up, but maybe we can proactively check what is the size of buf before calling send and raise an RuntimeException asking the user to reduce the number of metrics/datapoints.
Description:
Fixes issues described in issue #771 and issue #736
Todo:
StatsdMetricEmitterload-generatoris used (not part of this PR).By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.