-
Notifications
You must be signed in to change notification settings - Fork 36
add logs #1413
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
base: release/6.10
Are you sure you want to change the base?
add logs #1413
Changes from all commits
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 | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,10 +15,14 @@ | |||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import { PIPELINE_PROGRAMS_MAP } from 'gql/types/PipelineRecord/common'; | ||||||||||||||||||||||||||||||||||
| import log4js from 'log4js'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const log = log4js.getLogger('graphql'); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export async function pipelineRunsResolver(parent, args, context) { | ||||||||||||||||||||||||||||||||||
| const namespace = context.namespace; | ||||||||||||||||||||||||||||||||||
| const name = parent.name; | ||||||||||||||||||||||||||||||||||
| const queryId = context.queryId || 'Internal'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const pipelineType = parent.artifact.name || 'cdap-data-pipeline'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
@@ -30,11 +34,15 @@ export async function pipelineRunsResolver(parent, args, context) { | |||||||||||||||||||||||||||||||||
| programId: programId, | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| log.info(`[Query:${queryId}] Queueing programRuns load for pipeline: ${name}`); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const runInfo = await context.loaders.programRuns.load({ | ||||||||||||||||||||||||||||||||||
| namespace, | ||||||||||||||||||||||||||||||||||
| program, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| log.info(`[Query:${queryId}] Resolved programRuns for pipeline: ${name}`); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+37
to
+44
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. Since this resolver is executed for each individual pipeline in a list, logging at Since the DataLoader batch endpoint already logs the batched dispatch, we should change these individual queue/resolve logs to
Suggested change
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (!runInfo || (Array.isArray(runInfo) && runInfo.length === 0)) { | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,10 +15,14 @@ | |||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import { PIPELINE_PROGRAMS_MAP } from 'gql/types/PipelineRecord/common'; | ||||||||||||||||||||||||||||||||||
| import log4js from 'log4js'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const log = log4js.getLogger('graphql'); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export async function totalRunsResolvers(parent, args, context) { | ||||||||||||||||||||||||||||||||||
| const namespace = context.namespace; | ||||||||||||||||||||||||||||||||||
| const name = parent.name; | ||||||||||||||||||||||||||||||||||
| const queryId = context.queryId || 'Internal'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const pipelineType = parent.artifact.name || 'cdap-data-pipeline'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
@@ -30,11 +34,15 @@ export async function totalRunsResolvers(parent, args, context) { | |||||||||||||||||||||||||||||||||
| programId: programId, | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| log.info(`[Query:${queryId}] Queueing totalRuns load for pipeline: ${name}`); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const runInfo = await context.loaders.totalRuns.load({ | ||||||||||||||||||||||||||||||||||
| namespace, | ||||||||||||||||||||||||||||||||||
| program, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| log.info(`[Query:${queryId}] Resolved totalRuns for pipeline: ${name}`); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+37
to
+44
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. Since this resolver is executed for each individual pipeline in a list, logging at Since the DataLoader batch endpoint already logs the batched dispatch, we should change these individual queue/resolve logs to
Suggested change
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (!runInfo || !runInfo.runCount) { | ||||||||||||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
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.
Since this resolver is executed for each individual pipeline in a list, logging at$O(N)$ log entries (where $N$ is the number of pipelines). This can severely flood the application logs and degrade performance.
infolevel here will generateSince the DataLoader batch endpoint already logs the batched dispatch (which is much cleaner and grouped), we should change these individual queue/resolve logs to
debuglevel or remove them entirely.