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

server: make sure that the various replication loggers use consistent logging #8745

Merged
merged 2 commits into from
Sep 24, 2020

Conversation

rboyer
Copy link
Member

@rboyer rboyer commented Sep 24, 2020

This generic replicator code is used for 3 places: config entries, federation states, and enterprise stuff. This gets all 3 variants to actually log all of their components with the appropriate prefixes:

server.replication.config_entry: finished fetching config entries: amount=0
server.replication.config_entry: Config Entry replication: local=0 remote=0
server.replication.config_entry: Config Entry replication: deletions=0 updates=0
server.replication.config_entry: replication completed through remote index: index=1

server.replication.federation_state: finished fetching remote objects: amount=1
server.replication.federation_state: diffing replication state: local_amount=0 remote_amount=1
server.replication.federation_state: diffed replication state: deletions=0 updates=1
server.replication.federation_state: performing updates: updates=1
server.replication.federation_state: finished updates
server.replication.federation_state: replication completed through remote index: index=14
server.replication.federation_state: finished fetching remote objects: amount=2
server.replication.federation_state: diffing replication state: local_amount=1 remote_amount=2
server.replication.federation_state: diffed replication state: deletions=0 updates=1
server.replication.federation_state: performing updates: updates=1

@rboyer rboyer requested a review from a team September 24, 2020 18:54
@rboyer rboyer self-assigned this Sep 24, 2020
@@ -421,11 +421,11 @@ func NewServer(config *Config, flat Deps) (*Server, error) {
srv: s,
gatewayLocator: s.gatewayLocator,
},
Logger: s.logger,
Logger: s.loggers.Named(logging.Replication).Named(logging.FederationState),
Copy link
Member Author

Choose a reason for hiding this comment

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

A replicator instance takes the Logger on the ReplicatorConfig and does this Named+Named expansion before using it for all internal logging.

When using the function replicator style, the function accepts a logger as an argument, and that logger is the derived one with the Named+Named prefix above (this is what config entries do).

The federation states version uses the delegate style, and so has it's logger configured separately here. Since this was just using s.logger before it was not getting the Named+Named prefix.

Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM

@rboyer rboyer merged commit 0064f19 into master Sep 24, 2020
@rboyer rboyer deleted the fix-replicator-logger branch September 24, 2020 20:49
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.

2 participants