[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9157: Allow querying logs only by instance to simplify frontend query

[GitBox <gi...@apache.org>]

wu-sheng commented on code in PR #9157:
URL: https://github.com/apache/skywalking/pull/9157#discussion_r886409739


##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/OndemandLogQuery.java:
##########
@@ -34,64 +33,86 @@
 import com.google.gson.Gson;
 import org.apache.skywalking.oap.query.graphql.type.InternalLog;
 import org.apache.skywalking.oap.query.graphql.type.LogAdapter;
+import org.apache.skywalking.oap.query.graphql.type.OndemandContainergQueryCondition;
 import org.apache.skywalking.oap.query.graphql.type.OndemandLogQueryCondition;
-import org.apache.skywalking.oap.server.core.analysis.IDManager;
+import org.apache.skywalking.oap.server.core.analysis.manual.instance.InstanceTraffic.PropertyUtil;
 import org.apache.skywalking.oap.server.core.query.input.Duration;
+import org.apache.skywalking.oap.server.core.query.type.Attribute;
 import org.apache.skywalking.oap.server.core.query.type.Log;
 import org.apache.skywalking.oap.server.core.query.type.Logs;
+import org.apache.skywalking.oap.server.core.query.type.ServiceInstance;
 import org.apache.skywalking.oap.server.library.util.StringUtil;
 import graphql.kickstart.tools.GraphQLQueryResolver;
 import io.kubernetes.client.openapi.ApiException;
 import io.kubernetes.client.openapi.Configuration;
 import io.kubernetes.client.openapi.apis.CoreV1Api;
 import io.kubernetes.client.openapi.models.V1Container;
-import io.kubernetes.client.openapi.models.V1Namespace;
-import io.kubernetes.client.openapi.models.V1NamespaceList;
 import io.kubernetes.client.openapi.models.V1ObjectMeta;
 import io.kubernetes.client.openapi.models.V1Pod;
 import io.kubernetes.client.openapi.models.V1PodSpec;
 import io.kubernetes.client.util.Config;
+import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 
 @Slf4j
+@RequiredArgsConstructor
 public class OndemandLogQuery implements GraphQLQueryResolver {
     private final Gson gson = new Gson();
     private final Type responseType = new TypeToken<Map<String, Object>>() {
     }.getType();
     private CoreV1Api kApi;
 
-    public List<String> listNamespaces() throws IOException {
-        try {
-            final V1NamespaceList nsList =
-                kApi().listNamespace(null, null, null, null, null, null, null, null, null, null);
-            return nsList
-                .getItems()
-                .stream()
-                .map(V1Namespace::getMetadata)
-                .filter(Objects::nonNull)
-                .map(V1ObjectMeta::getName)
-                .collect(Collectors.toList());
-        } catch (ApiException e) {
-            log.error("Failed to list namespaces from Kubernetes, {}", e.getResponseBody(), e);
+    private final MetadataQueryV2 metadataQuery;
 
-            Map<String, Object> responseBody = gson.fromJson(e.getResponseBody(), responseType);
-            String message = responseBody.getOrDefault("message", e.getCode()).toString();
-            throw new RuntimeException(message);
-        }
+    public List<String> listContainers(final OndemandContainergQueryCondition condition)
+        throws IOException {
+        final ServiceInstance instance =
+            metadataQuery.getInstance(condition.getServiceInstanceId());
+        final Map<String, String> attributesMap = ensureInstanceHasPodProperties(instance);
+        final String ns = attributesMap.get(PropertyUtil.NAMESPACE);
+        final String pod = attributesMap.get(PropertyUtil.POD);
+        return listContainers(ns, pod);
     }
 
-    public List<String> listContainers(final OndemandLogQueryCondition condition)
+    public Logs ondemandPodLogs(final OndemandLogQueryCondition condition)
         throws IOException {
-        final String ns = condition.getNamespace();
-        final IDManager.ServiceInstanceID.InstanceIDDefinition instanceIDDefinition =
-            IDManager.ServiceInstanceID.analysisId(condition.getServiceInstanceId());
-        final String instanceName = instanceIDDefinition.getName();
+        final ServiceInstance instance =
+            metadataQuery.getInstance(condition.getServiceInstanceId());
+        final Map<String, String> attributesMap = ensureInstanceHasPodProperties(instance);
+        final String ns = attributesMap.get(PropertyUtil.NAMESPACE);
+        final String pod = attributesMap.get(PropertyUtil.POD);
+        return ondemandPodLogs(ns, pod, condition);
+    }
 
+    protected Map<String, String> ensureInstanceHasPodProperties(final ServiceInstance instance) {
+        if (instance == null) {
+            throw new IllegalArgumentException("No such instance");

Review Comment:
   Should we return an empty container list? rather than an exception?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org