-
Notifications
You must be signed in to change notification settings - Fork 26.1k
Add project routing telemetry infrastructure #155997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
a1b349f
af6076c
61213a7
d586afc
6e27857
8341614
6c956e3
e61f04e
2547bfa
95263f7
dec1b01
aff7bdc
4072e5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
|
|
||
| package org.elasticsearch.action.admin.cluster.stats; | ||
|
|
||
| import org.elasticsearch.TransportVersion; | ||
| import org.elasticsearch.action.admin.cluster.node.info.NodeInfo; | ||
| import org.elasticsearch.action.admin.cluster.node.stats.NodeStats; | ||
| import org.elasticsearch.action.admin.indices.stats.ShardStats; | ||
|
|
@@ -24,6 +25,8 @@ | |
|
|
||
| public class ClusterStatsNodeResponse extends BaseNodeResponse { | ||
|
|
||
| static final TransportVersion PROJECT_ROUTING_USAGE_STATS = TransportVersion.fromName("project_routing_usage_stats"); | ||
|
|
||
| private final NodeInfo nodeInfo; | ||
| private final NodeStats nodeStats; | ||
| private final ShardStats[] shardsStats; | ||
|
|
@@ -32,6 +35,7 @@ public class ClusterStatsNodeResponse extends BaseNodeResponse { | |
| private final RepositoryUsageStats repositoryUsageStats; | ||
| private final CCSTelemetrySnapshot searchCcsMetrics; | ||
| private final CCSTelemetrySnapshot esqlCcsMetrics; | ||
| private final ProjectRoutingUsageSnapshot projectRoutingUsageSnapshot; | ||
|
|
||
| public ClusterStatsNodeResponse(StreamInput in) throws IOException { | ||
| super(in); | ||
|
|
@@ -43,6 +47,9 @@ public ClusterStatsNodeResponse(StreamInput in) throws IOException { | |
| repositoryUsageStats = RepositoryUsageStats.readFrom(in); | ||
| searchCcsMetrics = new CCSTelemetrySnapshot(in); | ||
| esqlCcsMetrics = new CCSTelemetrySnapshot(in); | ||
| projectRoutingUsageSnapshot = in.getTransportVersion().supports(PROJECT_ROUTING_USAGE_STATS) | ||
| ? new ProjectRoutingUsageSnapshot(in) | ||
| : new ProjectRoutingUsageSnapshot(); | ||
| } | ||
|
|
||
| public ClusterStatsNodeResponse( | ||
|
|
@@ -54,7 +61,8 @@ public ClusterStatsNodeResponse( | |
| SearchUsageStats searchUsageStats, | ||
| RepositoryUsageStats repositoryUsageStats, | ||
| CCSTelemetrySnapshot ccsTelemetrySnapshot, | ||
| CCSTelemetrySnapshot esqlTelemetrySnapshot | ||
| CCSTelemetrySnapshot esqlTelemetrySnapshot, | ||
| ProjectRoutingUsageSnapshot projectRoutingUsageSnapshot | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to ensure non-null here? I think we are going the other route with tags... I am not a huge fan of nulls, but here I am wondering if we have nothing to display (which would be the case for all non-CPS cases I imagine) why create the object at all?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See this comment: #155997 (comment) |
||
| ) { | ||
| super(node); | ||
| this.nodeInfo = nodeInfo; | ||
|
|
@@ -65,6 +73,7 @@ public ClusterStatsNodeResponse( | |
| this.repositoryUsageStats = Objects.requireNonNull(repositoryUsageStats); | ||
| this.searchCcsMetrics = ccsTelemetrySnapshot; | ||
| this.esqlCcsMetrics = esqlTelemetrySnapshot; | ||
| this.projectRoutingUsageSnapshot = Objects.requireNonNull(projectRoutingUsageSnapshot); | ||
| } | ||
|
|
||
| public NodeInfo nodeInfo() { | ||
|
|
@@ -103,6 +112,10 @@ public CCSTelemetrySnapshot getEsqlCcsMetrics() { | |
| return esqlCcsMetrics; | ||
| } | ||
|
|
||
| public ProjectRoutingUsageSnapshot getProjectRoutingUsageSnapshot() { | ||
| return projectRoutingUsageSnapshot; | ||
| } | ||
|
|
||
| @Override | ||
| public void writeTo(StreamOutput out) throws IOException { | ||
| super.writeTo(out); | ||
|
|
@@ -114,6 +127,9 @@ public void writeTo(StreamOutput out) throws IOException { | |
| repositoryUsageStats.writeTo(out); | ||
| searchCcsMetrics.writeTo(out); | ||
| esqlCcsMetrics.writeTo(out); | ||
| if (out.getTransportVersion().supports(PROJECT_ROUTING_USAGE_STATS)) { | ||
| projectRoutingUsageSnapshot.writeTo(out); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As per above, maybe optional object here would be better, for non-CPS cases?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See this comment: #155997 (comment) |
||
| } | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| import org.elasticsearch.common.io.stream.StreamInput; | ||
| import org.elasticsearch.common.io.stream.StreamOutput; | ||
| import org.elasticsearch.common.unit.ByteSizeValue; | ||
| import org.elasticsearch.core.Nullable; | ||
| import org.elasticsearch.xcontent.ToXContentFragment; | ||
| import org.elasticsearch.xcontent.XContentBuilder; | ||
|
|
||
|
|
@@ -38,6 +39,9 @@ public class ClusterStatsResponse extends BaseNodesResponse<ClusterStatsNodeResp | |
| final RepositoryUsageStats repositoryUsageStats; | ||
| final CCSTelemetrySnapshot ccsMetrics; | ||
| final CCSTelemetrySnapshot esqlMetrics; | ||
| final ProjectRoutingUsageSnapshot projectRoutingUsageSnapshot; | ||
| @Nullable | ||
| final TagsConfigSnapshot tagsConfig; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As noted above, here it's a bit inconsistent - one is nullable, the other is not. Is there a reason why? The seem to be both non-existant in non-CPS context, or am I missing something?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree this is a little confusing. It is true that ProjectRoutingUsage and TagsConfig will only be present in serverless and both absent (in the _cluster/stats output) for stateful. The reason for the different handling here (one is null, the other is never null but can be full of zeros) is that:
Neither will "render" in _cluster/stats for stateful. For projectRoutingUsageSnapshot, it will not be included in the _cluster/stats output unless |
||
| final long timestamp; | ||
| final String clusterUUID; | ||
| private final Map<String, RemoteClusterStats> remoteClustersStats; | ||
|
|
@@ -56,7 +60,8 @@ public ClusterStatsResponse( | |
| VersionStats versionStats, | ||
| ClusterSnapshotStats clusterSnapshotStats, | ||
| Map<String, RemoteClusterStats> remoteClustersStats, | ||
| boolean skipMRT | ||
| boolean skipMRT, | ||
| @Nullable TagsConfigSnapshot tagsConfig | ||
| ) { | ||
| super(clusterName, nodes, failures); | ||
| this.clusterUUID = clusterUUID; | ||
|
|
@@ -65,6 +70,7 @@ public ClusterStatsResponse( | |
| indicesStats = new ClusterStatsIndices(nodes, mappingStats, analysisStats, versionStats); | ||
| ccsMetrics = new CCSTelemetrySnapshot(skipMRT == false); | ||
| esqlMetrics = new CCSTelemetrySnapshot(false); | ||
| projectRoutingUsageSnapshot = new ProjectRoutingUsageSnapshot(); | ||
| ClusterHealthStatus status = null; | ||
| for (ClusterStatsNodeResponse response : nodes) { | ||
| // only the master node populates the status | ||
|
|
@@ -76,9 +82,11 @@ public ClusterStatsResponse( | |
| nodes.forEach(node -> { | ||
| ccsMetrics.add(node.getSearchCcsMetrics()); | ||
| esqlMetrics.add(node.getEsqlCcsMetrics()); | ||
| projectRoutingUsageSnapshot.add(node.getProjectRoutingUsageSnapshot()); | ||
| }); | ||
| this.status = status; | ||
| this.clusterSnapshotStats = clusterSnapshotStats; | ||
| this.tagsConfig = tagsConfig; | ||
|
|
||
| this.repositoryUsageStats = nodes.stream() | ||
| .map(ClusterStatsNodeResponse::repositoryUsageStats) | ||
|
|
@@ -169,6 +177,20 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws | |
|
|
||
| builder.endObject(); | ||
|
|
||
| if (tagsConfig != null) { | ||
| builder.startObject("tags"); | ||
| tagsConfig.toXContent(builder, params); | ||
| builder.endObject(); | ||
| } | ||
|
|
||
| long totalQueries = projectRoutingUsageSnapshot.getSearchQueriesTotal() + projectRoutingUsageSnapshot.getEsqlQueriesTotal(); | ||
| if (totalQueries > 0) { | ||
| builder.startObject("project_routing"); | ||
| builder.field("queries", totalQueries); | ||
| projectRoutingUsageSnapshot.toXContent(builder, params); | ||
| builder.endObject(); | ||
| } | ||
|
|
||
| return builder; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| package org.elasticsearch.action.admin.cluster.stats; | ||
|
|
||
| import org.elasticsearch.cluster.ClusterState; | ||
| import org.elasticsearch.core.Nullable; | ||
|
|
||
| /** | ||
| * Extension point for supplying the {@code tags} configuration snapshot (tag names, named routing expressions, etc.) | ||
| * to {@code GET _cluster/stats}. Registered via {@link org.elasticsearch.plugins.ActionPlugin#getClusterStatsTagsProvider()}. | ||
| * | ||
| * <p>Needed as an extension point for serverless code. | ||
| */ | ||
| @FunctionalInterface | ||
| public interface ClusterStatsTagsProvider { | ||
|
|
||
| /** | ||
| * Returns the current tags configuration snapshot for the request's project, or {@code null} if the configuration | ||
| * is unavailable (e.g. CPS is disabled, or the project cannot be resolved from the thread context). | ||
| */ | ||
| @Nullable | ||
| TagsConfigSnapshot getTagsConfig(ClusterState clusterState); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| package org.elasticsearch.action.admin.cluster.stats; | ||
|
|
||
| import org.elasticsearch.core.Nullable; | ||
| import org.elasticsearch.search.crossproject.ProjectRoutingRequestInfo; | ||
|
|
||
| import java.util.concurrent.atomic.LongAdder; | ||
|
|
||
| /** | ||
| * Accumulates per-node project-routing telemetry counters. Follows the same pattern as {@link CCSUsageTelemetry}. | ||
| * Thread-safe via {@link LongAdder}. Obtain a point-in-time snapshot with {@link #getSnapshot()}. | ||
| * | ||
| * <p>All counters are gated on {@code hasLinkedProjects}: they only increment while the project has at least one | ||
| * configured linked project. This ensures percentages can be computed from the data | ||
| * (e.g. {@code queries_project_routing / queries}). | ||
| */ | ||
| public class ProjectRoutingUsageHolder { | ||
|
|
||
| // _search, _async_search, _msearch (per sub-request), _search/template, _msearch/template | ||
| private final LongAdder searchQueriesTotal = new LongAdder(); | ||
| private final LongAdder searchWithProjectRouting = new LongAdder(); | ||
| private final LongAdder searchWithAliasOrigin = new LongAdder(); | ||
| private final LongAdder searchWithAliasWildcard = new LongAdder(); | ||
| private final LongAdder searchWithCustomTags = new LongAdder(); | ||
| private final LongAdder searchWithNamedExpression = new LongAdder(); | ||
| private final LongAdder searchProjectRoutingFailures = new LongAdder(); | ||
|
|
||
| // ES|QL endpoint | ||
| private final LongAdder esqlQueriesTotal = new LongAdder(); | ||
| private final LongAdder esqlWithProjectRouting = new LongAdder(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels like search and ESQL counters both have common substructure:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good suggestion. Will be changed in the next push. |
||
| private final LongAdder esqlWithAliasOrigin = new LongAdder(); | ||
| private final LongAdder esqlWithAliasWildcard = new LongAdder(); | ||
| private final LongAdder esqlWithCustomTags = new LongAdder(); | ||
| private final LongAdder esqlWithNamedExpression = new LongAdder(); | ||
| private final LongAdder esqlWithSet = new LongAdder(); | ||
| private final LongAdder esqlProjectRoutingFailures = new LongAdder(); | ||
|
|
||
| /** | ||
| * Records a {@code _search} request. {@code queries} is always incremented (subject to the | ||
| * {@code hasLinkedProjects} gate). The {@code queries_project_routing} counter and its sub-counters are only | ||
| * incremented when {@code info} is non-null, i.e. the request carried a {@code project_routing} expression. | ||
| * | ||
| * @param info routing metadata from the resolver; null when the request had no {@code project_routing} header, | ||
| * or until the resolver is upgraded to populate it (Ticket 2) | ||
| * @param hasLinkedProjects true when the project had at least one linked project at the time of the request; | ||
| * when false all counters are skipped | ||
| */ | ||
| public void recordSearch(@Nullable ProjectRoutingRequestInfo info, boolean hasLinkedProjects) { | ||
| if (hasLinkedProjects == false) return; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe in this case we shouldn't even be calling it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The internal guard is intentional defensive design. |
||
| searchQueriesTotal.increment(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this is out of place for this patch, but I still feel this counter is in the wrong place. If we're interested in counting all search queries, we should be counting them on higher level, not inside routing counter. I am not sure this counter is even accurate here - we may have already ignored some queries because they didn't use routing before we came here. Unless I still misunderstand what counter means?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not counting all search queries. Our goal here is to count queries only those on projects where the origin project has at least one linked project. No other section of _cluster/stats will be interested in that metric. The Product Manager wants a sense of how many queries on "CPS projects" (those with at least one link) are using project routing. To do that we need to compute the "denominator", which is what this is.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK I see the point, but I think the wider point might still be - whether the cluster has any linked projects does not change much. Realistically, it almost never changes - once we got linked clusters, they will almost always stay, until the cluster is retired. And, when considering the projects to look at, we'd know which of them are CPS and which are not, and might just ignore the non-CPS ones. I feel it's a static project attribute more than a dynamic metric that needs to be continuously tracked. So if we just counted all the queries, and used it as denominator, instead of counting "all the queries while we had linked projects" I think it'd be just fine. And we already have
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No, that wouldn't work for the Product requirements. Product wants to know how project-routing is being used, including "how often" but only for "CPS projects" - projects with at least one 1 linked project. CPS projects is a subset, perhaps a small subset, of total serverless projects. If we counted all Search and ESQL queries as the denominator you can't then compute "what percentage of queries in CPS projects are using project routing?" as your denominator counter is for all projects. On a cluster with say 1000 customer projects where only 50 have linked projects, using the total count of all queries as denominator would wildly understate the adoption rate (e.g. 10% looks like 0.5% if you divide by 20x too many queries). |
||
| if (info == null) return; | ||
| searchWithProjectRouting.increment(); | ||
| if (info.usedAliasOrigin()) searchWithAliasOrigin.increment(); | ||
| if (info.usedAliasWildcard()) searchWithAliasWildcard.increment(); | ||
| if (info.usedNamedExpression()) searchWithNamedExpression.increment(); | ||
| if (info.tagsUsedInRouting().stream().anyMatch(t -> t.startsWith("_") == false)) searchWithCustomTags.increment(); | ||
| } | ||
|
|
||
| /** | ||
| * Records an ES|QL request. {@code queries} is always incremented (subject to the | ||
| * {@code hasLinkedProjects} gate). The {@code queries_project_routing} counter and its sub-counters are only | ||
| * incremented when {@code info} is non-null, i.e. the request carried a {@code project_routing} expression. | ||
| * | ||
| * @param info routing metadata from the resolver; null when the request had no {@code project_routing} expression, | ||
| * or until the resolver is upgraded to populate it (Ticket 2) | ||
| * @param setClauseUsed true when the routing expression came from an in-query {@code SET project_routing = ...} clause | ||
| * @param hasLinkedProjects true when the project had at least one linked project at the time of the request; | ||
| * when false all counters are skipped | ||
| */ | ||
| public void recordEsql(@Nullable ProjectRoutingRequestInfo info, boolean setClauseUsed, boolean hasLinkedProjects) { | ||
| if (hasLinkedProjects == false) return; | ||
| esqlQueriesTotal.increment(); | ||
| if (setClauseUsed) esqlWithSet.increment(); | ||
| if (info == null) return; | ||
| esqlWithProjectRouting.increment(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part seems to be copy-paste from the same part of the function above, maybe should be one function somehow?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will change with the change here. |
||
| if (info.usedAliasOrigin()) esqlWithAliasOrigin.increment(); | ||
| if (info.usedAliasWildcard()) esqlWithAliasWildcard.increment(); | ||
| if (info.usedNamedExpression()) esqlWithNamedExpression.increment(); | ||
| if (info.tagsUsedInRouting().stream().anyMatch(t -> t.startsWith("_") == false)) esqlWithCustomTags.increment(); | ||
| } | ||
|
|
||
| /** | ||
| * Records a routing failure for a {@code _search}-family request. Increments {@code queries}, | ||
| * {@code queries_project_routing}, and {@code failures}. Called by Ticket 5 from | ||
| * {@code AuthorizationService.onAuthorizedResourceLoadFailure()}. | ||
| * | ||
| * @param hasLinkedProjects true when the project had at least one linked project; when false this is a no-op | ||
| */ | ||
| public void recordSearchProjectRoutingFailure(boolean hasLinkedProjects) { | ||
| if (hasLinkedProjects == false) return; | ||
| searchQueriesTotal.increment(); | ||
| searchWithProjectRouting.increment(); | ||
| searchProjectRoutingFailures.increment(); | ||
| } | ||
|
|
||
| /** | ||
| * Records a routing failure for an ES|QL request. Increments {@code queries}, | ||
| * {@code queries_project_routing}, and {@code failures}. Called by Ticket 5 from | ||
| * {@code AuthorizationService.onAuthorizedResourceLoadFailure()}. | ||
| * | ||
| * @param hasLinkedProjects true when the project had at least one linked project; when false this is a no-op | ||
| */ | ||
| public void recordEsqlProjectRoutingFailure(boolean hasLinkedProjects) { | ||
| if (hasLinkedProjects == false) return; | ||
| esqlQueriesTotal.increment(); | ||
| esqlWithProjectRouting.increment(); | ||
| esqlProjectRoutingFailures.increment(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns a point-in-time snapshot of the current counters. | ||
| */ | ||
| public ProjectRoutingUsageSnapshot getSnapshot() { | ||
| return new ProjectRoutingUsageSnapshot( | ||
| searchQueriesTotal.sum(), | ||
| searchWithProjectRouting.sum(), | ||
| searchWithAliasOrigin.sum(), | ||
| searchWithAliasWildcard.sum(), | ||
| searchWithCustomTags.sum(), | ||
| searchWithNamedExpression.sum(), | ||
| searchProjectRoutingFailures.sum(), | ||
| esqlQueriesTotal.sum(), | ||
| esqlWithProjectRouting.sum(), | ||
| esqlWithAliasOrigin.sum(), | ||
| esqlWithAliasWildcard.sum(), | ||
| esqlWithCustomTags.sum(), | ||
| esqlWithNamedExpression.sum(), | ||
| esqlWithSet.sum(), | ||
| esqlProjectRoutingFailures.sum() | ||
| ); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is the best way - maybe we can do this on node construction and inject it into the usage service? Or is it not going to work for some reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed: switched to SPI registration via loadSingletonServiceProvider in NodeConstruction.createUsageService(), which resolves the provider at node construction time and injects it directly into UsageService's constructor. This also removed the volatile field and the registerTagsProvider() late-registration method.