Skip to content
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

Noise in the analysis logs #396

Closed
jgoizueta opened this issue Feb 12, 2020 · 4 comments
Closed

Noise in the analysis logs #396

jgoizueta opened this issue Feb 12, 2020 · 4 comments
Assignees

Comments

@jgoizueta
Copy link
Member

jgoizueta commented Feb 12, 2020

We have a function logError in the LimitsContext that expects as argument an err object with message and code properties.

But, in some cases, like when checking if limits are defined in the configuration (e.g. here), we're passing a text instead. So in such cases the text message is lost in the log and we only have a very generic "msg":"analysis:limits_error" that doesn't provide any useful information.

Since many of the analysis node limits are not defined in the configuration there's a lot of those messages the logs.

We should:

  • Fix the calls that pass a text, so it is not lost (e.g. context.logError({message: 'Limit for input rows is not defined'});).
  • Reconsider if the messages about limits not configured are useful, remove them otherwise
@dgaubert
Copy link
Contributor

cc @CartoDB/rt-managers for priorization

@ztephm
Copy link

ztephm commented Feb 12, 2020

Please prioritize this as important-level @juanrmn . Thank you.

@juanrmn juanrmn self-assigned this Feb 17, 2020
@juanrmn
Copy link
Member

juanrmn commented Feb 18, 2020

I've opened a PR (#399) about this. So far I've just fixed the calls to logError. About the usefulness of each message, I don't feel familiarized enough with the project to judge that, so all of them seem useful to me 🙂. However, if you think that any of them should be deleted, please add comments to the PR and I'll do it.

@dgaubert
Copy link
Contributor

dgaubert commented Mar 5, 2020

Done CartoDB/Windshaft-cartodb#1156

@dgaubert dgaubert closed this as completed Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants