Skip to content

Add ES logger#1209

Merged
alexshtin merged 2 commits into
temporalio:masterfrom
alexshtin:feature/add-es-logging
Jan 21, 2021
Merged

Add ES logger#1209
alexshtin merged 2 commits into
temporalio:masterfrom
alexshtin:feature/add-es-logging

Conversation

@alexshtin

@alexshtin alexshtin commented Jan 21, 2021

Copy link
Copy Markdown
Contributor

What changed?
Added error logger to ES client.

Why?
Improve visibility of ES errors.

How did you test it?
Run locally with simulated ES failures (ES is down).

Potential risks
No risks.

@alexshtin alexshtin marked this pull request as draft January 21, 2021 18:17
@alexshtin alexshtin force-pushed the feature/add-es-logging branch from d1f1d38 to 63fdb37 Compare January 21, 2021 18:47
@alexshtin alexshtin requested a review from wxing1292 January 21, 2021 18:49
@alexshtin alexshtin marked this pull request as ready for review January 21, 2021 18:49
elastic6.SetBasicAuth(config.Username, config.Password),

elastic6.SetErrorLog(newErrorLogger(logger)),
// elastic6.SetInfoLog(newInfoLogger(logger)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Debug code

elastic.SetBasicAuth(config.Username, config.Password),

elastic.SetErrorLog(newErrorLogger(logger)),
// elastic.SetInfoLog(newInfoLogger(logger)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove commented code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I want it to be there in case I will want to see info logs. There is not log level in ES. The only way I can control is to set these loggers or not.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 to Remove commented code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added clarifying comment.

@alexshtin alexshtin merged commit b46fbe3 into temporalio:master Jan 21, 2021
@alexshtin alexshtin deleted the feature/add-es-logging branch January 21, 2021 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants