Skip to content

Commit

Permalink
[Profiling] Rename stacktrace_ids field (elastic#105046)
Browse files Browse the repository at this point in the history 
With this commit we rename the field `stacktrace_ids` to
`stacktrace_ids_field` for the API calls `_profiling/stacktraces` and
`_profiling/flamegraph`. The semantics of this field are to contain the
name of the field to query in the indices provided by the `indices`
parameter. As the old field name was misleading (should we provide the
name of the field or a list of ids?) we rename it. As these APIs are
meant for exclusive use by Kibana and the field has been unused so far
we make this change directly without introducing any BWC layer.
  • Loading branch information
@danielmitterdorfer
danielmitterdorfer authored Feb 2, 2024
1 parent 7c2fdff commit c552bc1
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 19 deletions.
24 changes: 12 additions & 12 deletions ...ugin/profiling/src/main/java/org/elasticsearch/xpack/profiling/GetStackTracesRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class GetStackTracesRequest extends ActionRequest implements IndicesReque
public static final ParseField QUERY_FIELD = new ParseField("query");
public static final ParseField SAMPLE_SIZE_FIELD = new ParseField("sample_size");
public static final ParseField INDICES_FIELD = new ParseField("indices");
public static final ParseField STACKTRACE_IDS_FIELD = new ParseField("stacktrace_ids");
public static final ParseField STACKTRACE_IDS_FIELD = new ParseField("stacktrace_ids_field");
public static final ParseField REQUESTED_DURATION_FIELD = new ParseField("requested_duration");
public static final ParseField AWS_COST_FACTOR_FIELD = new ParseField("aws_cost_factor");
public static final ParseField CUSTOM_CO2_PER_KWH = new ParseField("co2_per_kwh");
Expand All @@ -49,7 +49,7 @@ public class GetStackTracesRequest extends ActionRequest implements IndicesReque
private QueryBuilder query;
private int sampleSize;
private String indices;
private String stackTraceIds;
private String stackTraceIdsField;
private Double requestedDuration;
private Double awsCostFactor;
private Double customCO2PerKWH;
Expand All @@ -73,7 +73,7 @@ public GetStackTracesRequest(
Double awsCostFactor,
QueryBuilder query,
String indices,
String stackTraceIds,
String stackTraceIdsField,
Double customCO2PerKWH,
Double customDatacenterPUE,
Double customPerCoreWattX86,
Expand All @@ -85,7 +85,7 @@ public GetStackTracesRequest(
this.awsCostFactor = awsCostFactor;
this.query = query;
this.indices = indices;
this.stackTraceIds = stackTraceIds;
this.stackTraceIdsField = stackTraceIdsField;
this.customCO2PerKWH = customCO2PerKWH;
this.customDatacenterPUE = customDatacenterPUE;
this.customPerCoreWattX86 = customPerCoreWattX86;
Expand Down Expand Up @@ -138,8 +138,8 @@ public String getIndices() {
return indices;
}

public String getStackTraceIds() {
return stackTraceIds;
public String getStackTraceIdsField() {
return stackTraceIdsField;
}

public boolean isAdjustSampleCount() {
Expand Down Expand Up @@ -170,7 +170,7 @@ public void parseXContent(XContentParser parser) throws IOException {
} else if (INDICES_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
this.indices = parser.text();
} else if (STACKTRACE_IDS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
this.stackTraceIds = parser.text();
this.stackTraceIdsField = parser.text();
} else if (REQUESTED_DURATION_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
this.requestedDuration = parser.doubleValue();
} else if (AWS_COST_FACTOR_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
Expand Down Expand Up @@ -215,14 +215,14 @@ public void parseXContent(XContentParser parser) throws IOException {
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (indices != null) {
if (stackTraceIds == null || stackTraceIds.isEmpty()) {
if (stackTraceIdsField == null || stackTraceIdsField.isEmpty()) {
validationException = addValidationError(
"[" + STACKTRACE_IDS_FIELD.getPreferredName() + "] is mandatory",
validationException
);
}
} else {
if (stackTraceIds != null) {
if (stackTraceIdsField != null) {
validationException = addValidationError(
"[" + STACKTRACE_IDS_FIELD.getPreferredName() + "] must not be set",
validationException
Expand Down Expand Up @@ -257,7 +257,7 @@ public String getDescription() {
// generating description lazily since the query could be large
StringBuilder sb = new StringBuilder();
appendField(sb, "indices", indices);
appendField(sb, "stacktrace_ids", stackTraceIds);
appendField(sb, "stacktrace_ids_field", stackTraceIdsField);
appendField(sb, "sample_size", sampleSize);
appendField(sb, "requested_duration", requestedDuration);
appendField(sb, "aws_cost_factor", awsCostFactor);
Expand Down Expand Up @@ -295,7 +295,7 @@ public boolean equals(Object o) {
return Objects.equals(query, that.query)
&& Objects.equals(sampleSize, that.sampleSize)
&& Objects.equals(indices, that.indices)
&& Objects.equals(stackTraceIds, that.stackTraceIds);
&& Objects.equals(stackTraceIdsField, that.stackTraceIdsField);
}

@Override
Expand All @@ -306,7 +306,7 @@ public int hashCode() {
// Resampler to produce a consistent downsampling results, relying on the default hashCode implementation of `query` will
// produce consistent results per node but not across the cluster. To avoid this, we produce the hashCode based on the
// string representation instead, which will produce consistent results for the entire cluster and across node restarts.
return Objects.hash(Objects.toString(query, "null"), sampleSize, indices, stackTraceIds);
return Objects.hash(Objects.toString(query, "null"), sampleSize, indices, stackTraceIdsField);
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...filing/src/main/java/org/elasticsearch/xpack/profiling/TransportGetStackTracesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ private void searchGenericEventGroupedByStackTrace(
new RandomSamplerAggregationBuilder("sample").setSeed(request.hashCode())
.setProbability(responseBuilder.getSamplingRate())
.subAggregation(
new CountedTermsAggregationBuilder("group_by").size(MAX_TRACE_EVENTS_RESULT_SIZE).field(request.getStackTraceIds())
new CountedTermsAggregationBuilder("group_by").size(MAX_TRACE_EVENTS_RESULT_SIZE)
.field(request.getStackTraceIdsField())
)
)
.execute(handleEventsGroupedByStackTrace(submitTask, client, responseBuilder, submitListener, searchResponse -> {
Expand Down
12 changes: 6 additions & 6 deletions ...profiling/src/test/java/org/elasticsearch/xpack/profiling/GetStackTracesRequestTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void testParseValidXContent() throws IOException {
assertEquals("@timestamp", ((RangeQueryBuilder) request.getQuery()).fieldName());
// Expect the default values
assertNull(request.getIndices());
assertNull(request.getStackTraceIds());
assertNull(request.getStackTraceIdsField());
assertNull(request.getAwsCostFactor());
assertNull(request.getCustomCO2PerKWH());
assertNull(request.getCustomDatacenterPUE());
Expand All @@ -66,7 +66,7 @@ public void testParseValidXContentWithCustomIndex() throws IOException {
.startObject()
.field("sample_size", 2000)
.field("indices", "my-traces")
.field("stacktrace_ids", "stacktraces")
.field("stacktrace_ids_field", "stacktraces")
.startObject("query")
.startObject("range")
.startObject("@timestamp")
Expand All @@ -83,7 +83,7 @@ public void testParseValidXContentWithCustomIndex() throws IOException {

assertEquals(2000, request.getSampleSize());
assertEquals("my-traces", request.getIndices());
assertEquals("stacktraces", request.getStackTraceIds());
assertEquals("stacktraces", request.getStackTraceIdsField());
// a basic check suffices here
assertEquals("@timestamp", ((RangeQueryBuilder) request.getQuery()).fieldName());

Expand Down Expand Up @@ -138,7 +138,7 @@ public void testParseValidXContentWithCustomCostAndCO2Data() throws IOException

// Expect the default values
assertNull(request.getIndices());
assertNull(request.getStackTraceIds());
assertNull(request.getStackTraceIdsField());
}
}

Expand Down Expand Up @@ -217,7 +217,7 @@ public void testValidateStacktraceWithoutIndices() {
);
List<String> validationErrors = request.validate().validationErrors();
assertEquals(1, validationErrors.size());
assertEquals("[stacktrace_ids] must not be set", validationErrors.get(0));
assertEquals("[stacktrace_ids_field] must not be set", validationErrors.get(0));
}

public void testValidateIndicesWithoutStacktraces() {
Expand All @@ -236,7 +236,7 @@ public void testValidateIndicesWithoutStacktraces() {
);
List<String> validationErrors = request.validate().validationErrors();
assertEquals(1, validationErrors.size());
assertEquals("[stacktrace_ids] is mandatory", validationErrors.get(0));
assertEquals("[stacktrace_ids_field] is mandatory", validationErrors.get(0));
}

public void testConsidersCustomIndicesInRelatedIndices() {
Expand Down

0 comments on commit c552bc1

Please sign in to comment.