Closed Bug 841457 Opened 7 years ago Closed 7 years ago

Convert stream logging to non-stream when stream operators are not used

Categories

(Core :: WebRTC: Signaling, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: abr, Assigned: abr)

References

Details

(Whiteboard: [WebRTC] [blocking-webrtc-] [qa-])

Attachments

(1 file, 3 obsolete files)

Several of the debug logging messages in the signaling subsystem use
the CSFLog(Critical|Error|Warn|Notice|Info|Debug)S format (with the
"S" at the end invoking the stream-based variation), but don't take
advantage of any stream features.
This patch is a mechanical search/replace to convert CSFLog*S calls
to CSFLog* equivalents everywhere streams are not used.
Attachment #714005 - Flags: review?(ethanhugg)
Severity: normal → enhancement
Priority: -- → P2
Whiteboard: [WebRTC] [blocking-webrtc-]
Attachment #714005 - Attachment is obsolete: true
Attachment #714005 - Flags: review?(ethanhugg)
Comment on attachment 714020 [details] [diff] [review]
Convert stream logging to non-stream when stream operators are not used

I noticed that another fairly large batch of these could be fixed by a different mechanical transform, so I updated the patch. FWIW, here is the regex transform I used:

> s/(CSFLog(Critical|Error|Warn|Notice|Info|Debug))S([^,]*?, ?)__FUNCTION__ << ":?([^<]*?)\); *([\r\n])/$1$3"%s:$4, __FUNCTION__);$5/s
Attachment #714020 - Flags: review?(ethanhugg)
Comment on attachment 714020 [details] [diff] [review]
Convert stream logging to non-stream when stream operators are not used

Review of attachment 714020 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm

::: media/webrtc/signaling/src/callcontrol/CallControlManagerImpl.cpp
@@ +37,5 @@
>    sipccLoggingMask(0),
>    authenticationStatus(AuthenticationStatusEnum::eNotAuthenticated),
>    connectionState(ConnectionStatusEnum::eIdle)
>  {
> +    CSFLogInfo(logTag, "CallControlManagerImpl()");

There are maybe a dozen of these that you might consider changing to __FUNCTION__ since you're in here.
Attachment #714020 - Flags: review?(ethanhugg) → review+
(In reply to Ethan Hugg [:ehugg] from comment #4)

> There are maybe a dozen of these that you might consider changing to
> __FUNCTION__ since you're in here.

Yeah, I'm trying to limit this to things I can do mechanically for this pass through. I'd like to treat manual fix-up as a follow-on task. I'm doing these changes as low-hanging-fruit so I can make progress on a suite of frustratingly intermittent bugs (see bug 841566).
Try push (along with an attempt to activate logging for test runs):
https://tbpl.mozilla.org/?tree=Try&rev=4247a24bb8e5
Blocks: 841496
Blocks: 839677
Blocks: 841150
The conversion turned up a compile error lying in wait for opt builds. Re-try:
https://tbpl.mozilla.org/?tree=Try&rev=a437dbcdc5f7
Attachment #714020 - Attachment is obsolete: true
Comment on attachment 714363 [details] [diff] [review]
Convert stream logging to non-stream when stream operators are not used

Carrying forward r+ from ehugg
Attachment #714363 - Flags: review+
Attachment #714363 - Attachment is obsolete: true
Comment on attachment 714366 [details] [diff] [review]
Convert stream logging to non-stream when stream operators are not used

Re-basing to tip, carrying forward Ethan's r+
Attachment #714366 - Flags: review+
Comment on attachment 714366 [details] [diff] [review]
Convert stream logging to non-stream when stream operators are not used

https://hg.mozilla.org/integration/mozilla-inbound/rev/954993f7a31c
Attachment #714366 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/954993f7a31c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Whiteboard: [WebRTC] [blocking-webrtc-] → [WebRTC] [blocking-webrtc-] [qa-]
You need to log in before you can comment on or make changes to this bug.