Skip to content
This repository has been archived by the owner on Sep 28, 2021. It is now read-only.

Access logging for HttpService #149

Merged
merged 14 commits into from
Aug 10, 2016
Merged

Conversation

pettermahlen
Copy link
Member

This PR does the following:

  • adds a generic solution for adding code to be notified of the outcome of a request
  • adds a default implementation that logs using the Apache HTTPD 'combined' format
  • ensures the HttpService uses this solution
  • fixes a bug in the Jetty HTTP server to ensure more requests are logged in case of failures.

@codecov-io
Copy link

codecov-io commented Aug 9, 2016

Current coverage is 69.02% (diff: 98.03%)

Merging #149 into 1.x will increase coverage by 0.63%

@@                1.x       #149   diff @@
==========================================
  Files           131        133     +2   
  Lines          2746       2793    +47   
  Methods           0          0          
  Messages          0          0          
  Branches        225        224     -1   
==========================================
+ Hits           1878       1928    +50   
+ Misses          824        822     -2   
+ Partials         44         43     -1   

Powered by Codecov. Last update b45f0ff...d650fb3

.filter(event -> event.getMessage().contains("Failed to write response"))
.collect(Collectors.toList());

assertThat(events.size(), is(1));
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use org.hamcrest.Matchers.hasSize (or even org.hamcrest.Matchers.contains) for sightly better error message

@jo-ri
Copy link
Contributor

jo-ri commented Aug 9, 2016

+1

@pettermahlen pettermahlen merged commit fad8d1f into spotify:1.x Aug 10, 2016
@pettermahlen pettermahlen deleted the http-logging-1.x branch August 10, 2016 07:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants