You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/01/04 04:28:19 UTC
[GitHub] [druid] themarcelor opened a new pull request #12116: Add http response status code to org.eclipse.jetty.server.RequestLog
themarcelor opened a new pull request #12116:
URL: https://github.com/apache/druid/pull/12116
### Description
This is a small but valuable change that augments the existing jetty logger instruction to present the HTTP response code.
This is extremely helpful to calculate the SR (Success Rate) based on the percentage of non-http-5xx requests, which powers SLO/SLA definitions and alerting (with a log indexing / metrics scrapping/exporting mechanism).
Please provide some handholding around testing, what is the best way to compile & dockerize this change so I can test this in my k8s cluster?
##### Key changed/added classes in this PR
* `JettyRequestLog.java`
<hr>
This PR has:
- [x] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on a change in pull request #12116: Add http response status code to org.eclipse.jetty.server.RequestLog
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on a change in pull request #12116:
URL: https://github.com/apache/druid/pull/12116#discussion_r777831037
##########
File path: server/src/main/java/org/apache/druid/server/initialization/jetty/JettyRequestLog.java
##########
@@ -35,11 +35,12 @@ public void log(Request request, Response response)
{
if (logger.isDebugEnabled()) {
logger.debug(
- "%s %s %s %s",
+ "%s %s %s %s %s",
Review comment:
the `getStatus` returns type of int, the format specifier here should be `%d`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] themarcelor edited a comment on pull request #12116: Add http response status code to org.eclipse.jetty.server.RequestLog
Posted by GitBox <gi...@apache.org>.
themarcelor edited a comment on pull request #12116:
URL: https://github.com/apache/druid/pull/12116#issuecomment-1006250922
👋🏼 Hi @FrankChen021 , do you know when the PRs get merged and a new release / docker image is published?
(looking forward to deploying this change into our k8s cluster so keeping an eye on https://hub.docker.com/r/apache/druid/tags)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 merged pull request #12116: Add http response status code to org.eclipse.jetty.server.RequestLog
Posted by GitBox <gi...@apache.org>.
FrankChen021 merged pull request #12116:
URL: https://github.com/apache/druid/pull/12116
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] themarcelor commented on pull request #12116: Add http response status code to org.eclipse.jetty.server.RequestLog
Posted by GitBox <gi...@apache.org>.
themarcelor commented on pull request #12116:
URL: https://github.com/apache/druid/pull/12116#issuecomment-1006250922
👋🏼 Hi @FrankChen021 , do you know when the PR might get merged and a new release / docker image will be published?
(looking forward to deploying this change into our k8s cluster so keeping an eye on https://hub.docker.com/r/apache/druid/tags)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] themarcelor commented on a change in pull request #12116: Add http response status code to org.eclipse.jetty.server.RequestLog
Posted by GitBox <gi...@apache.org>.
themarcelor commented on a change in pull request #12116:
URL: https://github.com/apache/druid/pull/12116#discussion_r778343636
##########
File path: server/src/main/java/org/apache/druid/server/initialization/jetty/JettyRequestLog.java
##########
@@ -35,11 +35,12 @@ public void log(Request request, Response response)
{
if (logger.isDebugEnabled()) {
logger.debug(
- "%s %s %s %s",
+ "%s %s %s %s %s",
Review comment:
Hi @FrankChen021 , is there a way to suppress the coverage failure? 🤷🏼
I have tried to deliver a silly unit test to avoid the lack of code coverage error but I can't allocate too much time for it atm.
Any help is deeply appreciated 🙏🏼
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on pull request #12116: Add http response status code to org.eclipse.jetty.server.RequestLog
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #12116:
URL: https://github.com/apache/druid/pull/12116#issuecomment-1006531263
The next release is going to be released in this month as planned. But I don't know the exact date.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org