[Infra UI] Add new graphql endpoint for snapshot data - #34264

Both nodes and metrics should use InfraSnapshotMetricResponse[] as the type. Also I would change the names to nodeBuckets and nodeBucketsWithMetrics.

They have different types, but you're right nevertheless.

Please change this to a map so that it doesn't side effect result and just return directly from the map. This will eliminate the any on line 186 as well.

How about renaming this to mergeNodeBuckets?

Lines 94-108 are identical to lines 164-178. How about we create a function named getAllNodeBuckets which return InfraSnapshotMetricResponse[]? I also think we need to defined the Aggregation response type and pass it to the second type argument on the function call, something like:

interface InfraSnapshotAggregation {
  buckets: InfraSnapshotMetricResponse[];
  after_key: { [id: string]: string };
}

interface InfraSnapshotAggregationResponse {
  nodes: InfraSnapshotAggregation;
}
const response = framework.callWithRequest<{}, InfraSnapshotAggregationResponse>(request, 'search', query);

When you provide an aggregation type, the generics that back up that request will return a type that defines the aggregation key as optional which will then require a check to make sure aggregations are return. We can't count on the response to always have that key set.

I would also like to see the let and side effecting while loop factored out of this code. If you combine lines 94-108 and 164-178 in to one function. You could write that function to recurse until there are no more results, then return all the buckets. Maybe something like this:

const getAllNodesBuckets = async (
  framework: InfraBackendFrameworkAdapter,
  req: InfraFrameworkRequest,
  requestQuery: InfraSnapshotNodeRequest, // this type needs to be created
  previousBuckets: InfraSnapshotMetricResponse[] = []
): Promise<InfraSnapshotMetricResponse[]> => {
  const response = await framework.callWithRequest<{}, InfraSnapshotAggregationResponse>(
    req,
    'search',
    requestQuery
  );

  // Nothing available, return the previous buckets.
  if (response.hits.total.value === 0) {
    return previousBuckets;
  }

    // if ES doesn't return aggregations key, we can either throw an error or return previousBuckets.
  if (!response.aggregations) {
    throw new Error('Whoops!, `aggregations` key must always be returned.');
  }

  const currentBuckets = response.aggregations.nodes.buckets;

  // if the buckets length is zero then we are finished paginating through the results
  if (currentBuckets.length === 0) {
    return previousBuckets;
  }

  // create a new request object
  const newQuery = {
    ...requestQuery,
    body: {
      ...requestQuery.body,
      aggs: {
        ...requestQuery.body.aggs,
        nodes: {
          ...requestQuery.body.aggs.nodes,
          composite: {
            ...requestQuery.body.aggs.nodes.composite,
            after: response.aggregations.nodes.after_key,
          },
        },
      },
    },
  };

  // request additional buckets
  return getAllNodesBuckets(framework, req, newQuery, [...previousBuckets, ...currentBuckets]);
};

Makes sense. Thanks.

How about we rename this to InfraSnapshotNodeBucket

Good point. I'll refine the type names in that file.

How about we use a map here with a trailing push for the last source definition and then return the results directly from the function? This needs a return type as well.

Agreed on the map, but I'm not sure about the return type -- this function is used only once and only factored out for readability. Adding an explicit type only adds mental overload here in my opinion.

I only recommended the type because Typescript will likely complain about the object/key signature. If it works without the type then it should be fine.

Change this to a map with a push for the trailing value and return directly from the function.

How about we use a reduce here to transform the array into a hash? Also we should define a return type for the parent function.

I would change groupby to groupBy to be consistent