Skip to content
This repository has been archived by the owner on Apr 11, 2023. It is now read-only.

Add Traces view to Service Details #1905

Merged
merged 1 commit into from Sep 4, 2020
Merged

Conversation

jotak
Copy link
Contributor

@jotak jotak commented Sep 2, 2020

Fixes kiali/kiali#3071

  • Make class AppTracesC more generic, allowing app/service/workload input (and rename it: TracesComponent)
  • TracesFetcher now fetches different endpoints depending on target kind
  • "JaegerItemC" now receives a "focusElement" instead of app (and is renamed: TraceDetails)
  • Remove now unused cleanServiceSelector function
  • Define server endpoints API for fetching service/workload based traces
  • Add Traces tab to ServiceDetailsPage

Backend PR: kiali/kiali#3169

@jotak jotak added this to In Review in Sprint 45 via automation Sep 2, 2020
@jotak
Copy link
Contributor Author

jotak commented Sep 2, 2020

WIP: server-side pending work done

Fixes kiali/kiali#3071

- Make class AppTracesC more generic, allowing app/service/workload input (and rename it: TracesComponent)
- TracesFetcher now fetches different endpoints depending on target kind
- "JaegerItemC" now receives a "focusElement" instead of app (and is renamed: TraceDetails)
- Remove now unused cleanServiceSelector function
- Define server endpoints API for fetching service/workload based traces
- Add Traces tab to ServiceDetailsPage
Copy link
Contributor

@hhovsepy hhovsepy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified:
Screenshot from 2020-09-03 14-58-17

jaegerTraces: (namespace: string, app: string) => `api/namespaces/${namespace}/apps/${app}/traces`,
appTraces: (namespace: string, app: string) => `api/namespaces/${namespace}/apps/${app}/traces`,
serviceTraces: (namespace: string, svc: string) => `api/namespaces/${namespace}/services/${svc}/traces`,
workloadTraces: (namespace: string, wkd: string) => `api/namespaces/${namespace}/workloads/${wkd}/traces`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not yet in the backend, isn't it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I'm a bit ahead of time for this one ... first i thought i would do both tasks at the same time..
I can remove if you prefer, but anyway, I'll start to work on the workload case right after that one

return newRequest<JaegerResponse>(HTTP_VERBS.GET, urls.serviceTraces(namespace, service), params, {});
};

export const getWorkloadTraces = (namespace: string, workload: string, params: TracingQuery) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is left here, this endpoint is not yet in the backend.

Copy link
Contributor

@lucasponce lucasponce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frontend code looks ok.
I have a couple of comments in the backend code, but I've tested locally and it seems ok.

@lucasponce lucasponce merged commit e80ca7b into kiali:master Sep 4, 2020
Sprint 45 automation moved this from In Review to Done Sep 4, 2020
@ghost ghost added this to the v1.24.0 milestone Sep 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Sprint 45
  
Done
3 participants