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

Add RequestLogEventBuilderFactory #6477

Merged
merged 4 commits into from
Oct 31, 2018

Conversation

leventov
Copy link
Member

This PR allows to control the fields in RequestLogEvent, emitted in EmittingRequestLogger. In our case, we want to get rid of the intervals fields of the query objects that are a part of DefaultRequestLogEvent. They are enormous (thousands of segments) and not useful.

Related to #5522, FYI @a2l007.

@leventov leventov requested a review from jon-wei October 22, 2018 16:35
@@ -56,96 +51,7 @@ public String toString()
return "EmittingRequestLogger{" +
"emitter=" + emitter +
", feed='" + feed + '\'' +
", requestLogEventBuilderFactory=" + requestLogEventBuilderFactory +
Copy link
Contributor

Choose a reason for hiding this comment

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

DefaultRequestLogEventBuilderFactory.toString() is not implemented.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed

Copy link
Contributor

@jon-wei jon-wei left a comment

Choose a reason for hiding this comment

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

LGTM

@leventov leventov added the WIP label Oct 30, 2018
@leventov leventov removed the WIP label Oct 31, 2018
@jihoonson
Copy link
Contributor

LGTM after travis.

@leventov leventov merged commit 2cdce2e into apache:master Oct 31, 2018
@leventov leventov deleted the RequestLogEventBuilderFactory branch October 31, 2018 21:24
@jon-wei jon-wei added this to the 0.14.0 milestone Feb 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants