Skip to content

Commit

Permalink
[Profiling] Use seed for consistent test results
Browse files Browse the repository at this point in the history
In some cases (APM integration) profiling APIs may use the random
sampler aggregation which inherently produces random results. To have
consistent results in tests we have implemented a hack that kept the
seed used by random sampler constant. With elastic#104830 it is now possible to
provide the shard seed directly to random sampler so we can now remove
this hack.

Relates elastic#104830
  • Loading branch information
danielmitterdorfer committed Feb 23, 2024
1 parent 1dd2712 commit eb8bf90
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@

package org.elasticsearch.xpack.profiling;

import org.elasticsearch.index.Index;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.TermQueryBuilder;
import org.elasticsearch.index.shard.ShardId;

import java.util.List;

Expand Down Expand Up @@ -92,7 +90,6 @@ public void testGetStackTracesFromAPMWithMatchNoDownsampling() throws Exception

public void testGetStackTracesFromAPMWithMatchAndDownsampling() throws Exception {
TermQueryBuilder query = QueryBuilders.termQuery("transaction.name", "encodeSha1");
Index apmTest = resolveIndex("apm-test-001");

GetStackTracesRequest request = new GetStackTracesRequest(
1,
Expand All @@ -107,28 +104,9 @@ public void testGetStackTracesFromAPMWithMatchAndDownsampling() throws Exception
null,
null,
null
) {
@Override
public boolean equals(Object o) {
return super.equals(o);
}

@Override
public int hashCode() {
// The random sampler aggregation takes a user-provided seed as well as the index UUID into account for randomness. This is
// fine for a production use case but here we need full control over the internal seed so test results are stable. As
// the index UUID changes between test runs, and we have no control over it, we will instead modify the user provided seed
// so that the random number generator is always initialized the same, regardless of the index UUID.
//
// See org.elasticsearch.search.aggregations.bucket.sampler.random.RandomSamplingQuery#createWeight(), specifically the
// initialization of SplittableRandom(), which uses both the "seed" (user-provided) and a "hash", which is built from
// ShardId#hashCode(). By using the same hash code, the XOR will always evaluate to 0, thus producing a consistent seed for
// SplittableRandom().
int baseSeed = new ShardId(apmTest, 0).hashCode();
// a seed of zero won't return results for our test scenario, so we toggle one bit to generate a consistent non-zero seed.
return baseSeed ^ 2;
}
};
);
// ensures consistent results in the random sampler aggregation that is used internally
request.setShardSeed(42);

GetStackTracesResponse response = client().execute(GetStackTracesAction.INSTANCE, request).get();
assertEquals(49, response.getTotalFrames());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ public class GetStackTracesRequest extends ActionRequest implements IndicesReque
// sample counts by default and remove this flag.
private Boolean adjustSampleCount;

// This is only meant for testing and is intentionally not exposed in the REST API.
private Integer shardSeed;

public GetStackTracesRequest() {
this(null, null, null, null, null, null, null, null, null, null, null, null);
}
Expand Down Expand Up @@ -167,6 +170,14 @@ public void setAdjustSampleCount(Boolean adjustSampleCount) {
this.adjustSampleCount = adjustSampleCount;
}

public Integer getShardSeed() {
return shardSeed;
}

public void setShardSeed(Integer shardSeed) {
this.shardSeed = shardSeed;
}

public void parseXContent(XContentParser parser) throws IOException {
XContentParser.Token token = parser.currentToken();
String currentFieldName = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,16 @@ private void searchGenericEventGroupedByStackTrace(
ActionListener<GetStackTracesResponse> submitListener,
GetStackTracesResponseBuilder responseBuilder
) {

RandomSamplerAggregationBuilder randomSampler = new RandomSamplerAggregationBuilder("sample").setSeed(request.hashCode())
.setProbability(responseBuilder.getSamplingRate())
.subAggregation(
new CountedTermsAggregationBuilder("group_by").size(MAX_TRACE_EVENTS_RESULT_SIZE).field(request.getStackTraceIdsField())
);
// shard seed is only set in tests and ensures consistent results
if (request.getShardSeed() != null) {
randomSampler.setShardSeed(request.getShardSeed());
}
client.prepareSearch(request.getIndices())
.setTrackTotalHits(false)
.setSize(0)
Expand All @@ -266,14 +276,7 @@ private void searchGenericEventGroupedByStackTrace(
.setQuery(request.getQuery())
.addAggregation(new MinAggregationBuilder("min_time").field("@timestamp"))
.addAggregation(new MaxAggregationBuilder("max_time").field("@timestamp"))
.addAggregation(
new RandomSamplerAggregationBuilder("sample").setSeed(request.hashCode())
.setProbability(responseBuilder.getSamplingRate())
.subAggregation(
new CountedTermsAggregationBuilder("group_by").size(MAX_TRACE_EVENTS_RESULT_SIZE)
.field(request.getStackTraceIdsField())
)
)
.addAggregation(randomSampler)
.execute(handleEventsGroupedByStackTrace(submitTask, client, responseBuilder, submitListener, searchResponse -> {
long totalSamples = 0;
SingleBucketAggregation sample = searchResponse.getAggregations().get("sample");
Expand Down

0 comments on commit eb8bf90

Please sign in to comment.