Improve IF logging protocols #753

manushak · 2024-05-31T11:54:39Z

Types of changes

Enhancement (project structure, spelling, grammar, formatting)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

A description of the changes proposed in the Pull Request

add --debug command functionality
add debug log messages into the codebase accordingly

jmcook1186

Seems like something is not right here, see these logs for example:

DEBUG: 2024-05-31T12:29:04.237Z: teads-curve: Computing pipeline for `sum`
DEBUG: 2024-05-31T12:29:04.238Z: sum: Computing pipeline for `coefficient`
DEBUG: 2024-05-31T12:29:04.238Z: coefficient: Computing pipeline for `multiply`

the plugin names do not match (e.g. teads-curve: Computing pipeline for sum)

Also I don't see the console.x rerouting working. Can you confirm?

jmcook1186

lgtm - tested with two manifests from our examples folder and saw the logs I exected when running using --debug. Also tried injecting some dummy log statements into the builtins and saw their messages get emitted via the IF logger during the execution, which is great.

Thanks

src/lib/parameterize.ts

narekhovhannisyan · 2024-06-03T13:49:21Z

this log appear whenever if is executed without adding --debug flag.

narekhovhannisyan · 2024-06-03T13:51:59Z

src/index.ts

 import {parseIEProcessArgs} from './util/args';
 import {andHandle} from './util/helpers';
 import {logger} from './util/logger';
 import {validateManifest} from './util/validations';

 import {STRINGS} from './config';

-const {DISCLAIMER_MESSAGE} = STRINGS;
+const {DISCLAIMER_MESSAGE, STARTING_IMPACT_FRAMEWORK, EXITING_IF} = STRINGS;


if exiting if, then starting if

@manushak you got this wrong. in starting you have written impact framework, however in exiting just if. So for fixing this inconsistency please use if key in both places.

Uh, I've got it now. The variable names are consistent with the message itself.
@jmcook1186, maybe we need to rename 'Starting Impact framework' to 'Starting IF' to be consistent with 'Exiting IF'. What do you think?

@narekhovhannisyan renamed

jmcook1186 · 2024-06-03T13:54:42Z

hi @manushak I found a problem wit this PR - it doesn't output to --stdout unless --debug is raised.

e.g. npm run ie -- -m manifests/plugins/sci/success.yml --stdout

prints nothing unless you add --debug, in which case you get the output in the console.

src/util/debug-logger.ts

MariamKhalatova

@manushak
Syncing prameters is shown at the beginning of the log and without DEBUG label. So no matter running with --debug or without it. The console.debug(SYNCING_PARAMETERS); was called at wrong place

MariamKhalatova · 2024-06-08T08:44:15Z

@manushak we are getting ton of checking aggregation method logs. Each field should appear once.
mock-obs-time-sync.yml was used to reproduce.

MariamKhalatova · 2024-06-08T09:47:55Z

@manushak when running if with debug flag, exhaust's logs are gone under INFO: .

manushak · 2024-06-10T05:42:25Z

@manushak we are getting ton of checking aggregation method logs. Each field should appear once. mock-obs-time-sync.yml was used to reproduce.

@MariamKhalatova we get these huge logs because it traverses every input field.
@jmcook1186 Wdyt is this the right behavior?

MariamKhalatova · 2024-06-10T07:58:31Z

@manushak It's aggregation log and it's called repetitively for each input member (imagine what would happen with complex pipelines of 1000 nodes). There was memoize log function in code base, you can use that to log each one once.

jmcook1186 · 2024-06-10T08:23:19Z

@manushak @MariamKhalatova I think the aggregation check only needs to be displayed one time for each parameter (i.e. for each element in inputs[0]), as the operation is entirely idempotent - nothing can change between inputs[i] and inputs[i+1]...inputs[N] (or at least the probability is so low that it is not worth having the additional logs).

manushak · 2024-06-11T04:03:26Z

@MariamKhalatova, I've fixed. Please test

narekhovhannisyan · 2024-06-11T05:35:23Z

src/lib/parameterize.ts

+  /**
+   * Creates a handler for managing unique unit names.
+   */
+  const setUniqueUnitName = (() => {


we already have something like this in our codebase memoizedLog

you can reuse that with minor modifications for avoiding code duplication

…nsole.log type

jmcook1186 · 2024-06-11T13:47:14Z

what's the status of this @manushak @MariamKhalatova ? have @MariamKhalatova 's requested changes been addressed?

manushak · 2024-06-11T13:54:46Z

what's the status of this @manushak @MariamKhalatova ? have @MariamKhalatova 's requested changes been addressed?

the requested changes have been fixed, and are waiting for testing

manushak added 11 commits May 30, 2024 19:22

feat(util): add debug logging functionality

848e42d

docs(util): add debug command description

7c6a273

fix(util): remove unnecessary debugLog type handle

91b9efe

test(util): add debug-logger test

bc8a364

feat(src): add debug log in the index file

bf708da

feat(builtins): add debug log in builtin files

e8210bd

feat(config): add debug log messages and add 'debug' command description

b899631

feat(lib): add debug log into the lib files

2218776

feat(types): add debug key into IEArgs and ProcessArgsOutputs interfaces

61500a5

feat(util): add debug logs into validations.ts file

9b0b3ac

feat(util): add debug command into 'parseIEProcessArgs' function

ad4b46b

manushak added feature labels May 31, 2024

manushak requested review from narekhovhannisyan and jmcook1186 May 31, 2024 11:54

manushak self-assigned this May 31, 2024

manushak linked an issue May 31, 2024 that may be closed by this pull request

Improve IF logging protocols #600

Closed

5 tasks

jmcook1186 requested changes May 31, 2024

View reviewed changes

fix(lib): fix debug logging for plugins

4a57fb7

manushak requested a review from jmcook1186 May 31, 2024 13:09

jmcook1186 approved these changes May 31, 2024

View reviewed changes

narekhovhannisyan reviewed May 31, 2024

View reviewed changes

src/lib/parameterize.ts Outdated Show resolved Hide resolved

narekhovhannisyan reviewed May 31, 2024

View reviewed changes

src/lib/parameterize.ts Outdated Show resolved Hide resolved

fix(lib): uncomment debug log message

9d67d90

manushak requested a review from narekhovhannisyan May 31, 2024 18:46

narekhovhannisyan reviewed Jun 3, 2024

View reviewed changes

src/util/debug-logger.ts Outdated Show resolved Hide resolved

zanete removed feature labels Jun 5, 2024

jmcook1186 mentioned this pull request Jun 5, 2024

Impact Framework Project Updates2024-06-05 #780

Open

fix(src): rename STARTING_IMPACT_FRAMEWORK to STARTING_IF

2810b53

narekhovhannisyan approved these changes Jun 6, 2024

View reviewed changes

MariamKhalatova requested changes Jun 6, 2024

View reviewed changes

fix(lib): move debug log to the right place

1b13600

manushak requested a review from MariamKhalatova June 7, 2024 14:19

manushak added 2 commits June 10, 2024 10:49

fix(util): fix manifest output log

3c85e92

test(util): add test coverage

567ed4e

manushak added 2 commits June 10, 2024 21:12

fix(lib): print unit names only for the first input

4317ae4

test(lib): add test coverage

f57f119

narekhovhannisyan reviewed Jun 11, 2024

View reviewed changes

manushak added 4 commits June 11, 2024 16:26

feat(util): update memoizedLogger function to accept property with co…

5c7b8eb

…nsole.log type

fix(lib): instead of setUniqueUnitName function use memoizedLog

8c6181d

test(lib): update test

ed34290

Merge branch 'main' into debug-logger

0784ca5

MariamKhalatova approved these changes Jun 11, 2024

View reviewed changes

MariamKhalatova merged commit 95e9526 into main Jun 11, 2024
2 checks passed

MariamKhalatova deleted the debug-logger branch June 11, 2024 16:56

This was referenced Jun 11, 2024

Release v0.4.0-beta.0 #806

Merged

Release v0.4.0 #836

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve IF logging protocols #753

Improve IF logging protocols #753

manushak commented May 31, 2024

jmcook1186 left a comment •

edited

Loading

jmcook1186 left a comment

narekhovhannisyan commented Jun 3, 2024

narekhovhannisyan Jun 3, 2024 •

edited

Loading

narekhovhannisyan Jun 3, 2024 •

edited

Loading

manushak Jun 4, 2024

manushak Jun 6, 2024

jmcook1186 commented Jun 3, 2024 •

edited

Loading

MariamKhalatova left a comment •

edited

Loading

MariamKhalatova commented Jun 8, 2024 •

edited by manushak

Loading

MariamKhalatova commented Jun 8, 2024

manushak commented Jun 10, 2024

MariamKhalatova commented Jun 10, 2024 •

edited

Loading

jmcook1186 commented Jun 10, 2024

manushak commented Jun 11, 2024

narekhovhannisyan Jun 11, 2024 •

edited

Loading

jmcook1186 commented Jun 11, 2024

manushak commented Jun 11, 2024

Improve IF logging protocols #753

Improve IF logging protocols #753

Conversation

manushak commented May 31, 2024

Types of changes

A description of the changes proposed in the Pull Request

jmcook1186 left a comment • edited Loading

Choose a reason for hiding this comment

jmcook1186 left a comment

Choose a reason for hiding this comment

narekhovhannisyan commented Jun 3, 2024

narekhovhannisyan Jun 3, 2024 • edited Loading

Choose a reason for hiding this comment

narekhovhannisyan Jun 3, 2024 • edited Loading

Choose a reason for hiding this comment

manushak Jun 4, 2024

Choose a reason for hiding this comment

manushak Jun 6, 2024

Choose a reason for hiding this comment

jmcook1186 commented Jun 3, 2024 • edited Loading

MariamKhalatova left a comment • edited Loading

Choose a reason for hiding this comment

MariamKhalatova commented Jun 8, 2024 • edited by manushak Loading

MariamKhalatova commented Jun 8, 2024

manushak commented Jun 10, 2024

MariamKhalatova commented Jun 10, 2024 • edited Loading

jmcook1186 commented Jun 10, 2024

manushak commented Jun 11, 2024

narekhovhannisyan Jun 11, 2024 • edited Loading

Choose a reason for hiding this comment

jmcook1186 commented Jun 11, 2024

manushak commented Jun 11, 2024

jmcook1186 left a comment •

edited

Loading

narekhovhannisyan Jun 3, 2024 •

edited

Loading

narekhovhannisyan Jun 3, 2024 •

edited

Loading

jmcook1186 commented Jun 3, 2024 •

edited

Loading

MariamKhalatova left a comment •

edited

Loading

MariamKhalatova commented Jun 8, 2024 •

edited by manushak

Loading

MariamKhalatova commented Jun 10, 2024 •

edited

Loading

narekhovhannisyan Jun 11, 2024 •

edited

Loading