Skip to content

Logging#326

Closed
alanconway wants to merge 3 commits intoapache:masterfrom
alanconway:logging
Closed

Logging#326
alanconway wants to merge 3 commits intoapache:masterfrom
alanconway:logging

Conversation

@alanconway
Copy link
Copy Markdown
Contributor

Some minor changes to dispatch logging based on experience debugging the HTTP Envoy bridge.
See the commit comments for explanation.

One additional question: I was confused by the new messageLoggingComponents config, when I set MESSAGE enable trace+ I saw nothing at all, which was surprising. Perhaps the default should be "all" so that enabling MESSAGE trace does what folks expect and they can turn the logging down per connector/listener if they wish. Since trace logging will be off in production (I hope!) this won't have any effect except on users that are deliberately debugging AND have turned on trace logging for at least the MESSAGE module, so are expecting a lot of log data.

- upgrade connection failure INFO messages to WARNING
- downgrade incref/decref messages to TRACE

Personally I think the incref/decref messages should not be in user-facing logs
at all, but if they must be they should be TRACE as they occur at a per-delivery
(i.e. per-message) volume.
…ection

Incoming connection establishment was being logged at info.
Added info logs for outgoing connection establishment and any connection close.
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #326 into master will increase coverage by 0.02%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #326      +/-   ##
==========================================
+ Coverage   86.56%   86.59%   +0.02%     
==========================================
  Files          69       69              
  Lines       15467    15471       +4     
==========================================
+ Hits        13389    13397       +8     
+ Misses       2078     2074       -4
Impacted Files Coverage Δ
src/router_core/transfer.c 90.59% <100%> (ø) ⬆️
src/server.c 87.78% <87.5%> (+0.04%) ⬆️
src/iterator.c 86.6% <0%> (-0.2%) ⬇️
src/router_core/router_core.c 99.25% <0%> (+0.37%) ⬆️
src/router_core/agent_link.c 63.79% <0%> (+0.57%) ⬆️
src/log.c 91.22% <0%> (+1.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d5d7cd...d92719a. Read the comment docs.


qd_log(ctx->server->log_source, QD_LOG_INFO, "Accepted connection to %s from %s",
name, ctx->rhost_port);
qd_log(ctx->server->log_source, QD_LOG_INFO,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There appear to be more arguments than format directives in this call.
Why did you remove the information about the remote host?

@alanconway alanconway closed this Aug 6, 2018
@alanconway alanconway deleted the logging branch August 6, 2018 22:08
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