[SERVER-52604] Log from anywhere (normalize MONGO_LOGV2_DEFAULT_COMPONENT) Created: 03/Nov/20  Updated: 29/Oct/23  Resolved: 04/May/22

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: 6.1.0-rc0

Type: Improvement Priority: Major - P3
Reporter: Billy Donahue Assignee: Billy Donahue
Resolution: Fixed Votes: 1
Labels: sa-remove-fv-backlog-22
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
depends on SERVER-54596 1-arg shouldLog violates One Definiti... Closed
Issue split
split to SERVER-66202 MONGO_LOGV2_DEFAULT_COMPONENT convent... Closed
split to SERVER-66203 Sweep: move MONGO_LOGV2_DEFAULT_COMPO... Closed
Related
related to SERVER-80847 reinforce MONGO_LOGV2_DEFAULT_COMPONE... Closed
Backwards Compatibility: Fully Compatible
Sprint: Service Arch 2022-05-16
Participants:

 Description   

The #define MONGO_LOGV2_DEFAULT_COMPONENT definition is well-known to be a C++ design problem. It has to appear before including headers, etc.

This inverted dependency of a header upon its includer affects considerations of precompiled headers(PM-2150) or C++ modules(PM-2149).

 

All we have to do is move that definition down below the include files, like any other local macro, and it's much more conventional C++.

If you want to define it in a header and do some LOGV2 logging from inline functions, that's ok, just #undef it at the bottom. There is no ODR problem if an inline function has LOGV2 statements, as long as it always contains the same body, which it will. Nothing bad happens.

 

log.h can switch from asserting that the macro IS defined to asserting that the macro is NOT defined when log.h is included.

 

 

#define MONGO_LOGV2_DEFAULT_COMPONENT x
#include "mongo/logv2/log.h"
...

to

#include "mongo/logv2/log.h"
...
#define MONGO_LOGV2_DEFAULT_COMPONENT x

Then we don't have a #define that must appear before header includes.

 

A side-effect of this is change is that shouldLog's component argument becomes mandatory. This is easy to do and only affects 5 or 6 calls.



 Comments   
Comment by Githook User [ 04/May/22 ]

Author:

{'name': 'Billy Donahue', 'email': 'billy.donahue@mongodb.com', 'username': 'BillyDonahue'}

Message: SERVER-52604 normalize log.h inclusion semantics
Branch: master
https://github.com/mongodb/mongo/commit/1634edc01f2fc41d1a4e61f3802c5cec90e0e793

Comment by Billy Donahue [ 17/Feb/21 ]

Long Weekend PoC.

https://github.com/mongodb/mongo/compare/master...BillyDonahue:SERVER-52604_undefs

Putting it on Github because it would be hard to see the staged progression in mongodbcr.

 

 

It's less change than I thought, outside logv2/log.h.

The shouldLog function now has a mandatory component. You can give MONGO_LOGV2_DEFAULT_COMPONENT to it though.
So there used to be shouldLog(component,severity) and shouldlog(severity), but that second one has to go.

Some code that was logging from headers (bending the "rule!") had to change a little.

 

The codebase really just needs to move all of its #define MONGO_LOGV2_DEFAULT_COMPONENT down below the header includes, and proceed normally.

If a .h file wants to log, it can #define the MONGO_LOGV2_DEFAULT_COMPONENT just like a .cpp file would, and then #undef it at the bottom.
Nothing bad happens if we do this.

 

To be clear, I have already done this and it works.

The tool that rewrites the codebase to place the macro after the include blocks is here:

https://gist.github.com/BillyDonahue/48c2746b9a761e1d1ecc930486d83aa5

Generated at Thu Feb 08 05:28:30 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.