841641 - make it easier to generate production builds with WebRTC signaling logging turned on

Status: RESOLVED FIXED

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

Description

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP force on logging for the signalling code

This patch is currently against the alder branch to help us debug performance issues. It's potentially also worth turning on all the time so that we can ask nightly or even release users, devs, customers, etc to generate and send us logs when problems arise. 

It'll need a bit of tweaking before it lands, for sure.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

abr or ehugg would be fine

Comment 2

[Ethan Hugg [:ehugg]]

Comment on attachment 714213 [details] [diff] [review]
WIP force on logging for the signalling code

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

I will post an updated patch and try build soon.

::: media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
@@ +4,5 @@
>  
>  #ifndef MEDIA_CONDUIT_ABSTRACTION_
>  #define MEDIA_CONDUIT_ABSTRACTION_
>  
> +#define FORCE_PR_LOG

I think I'm going to change this to an include of CSFLog.h so that FORCE_PR_LOG is in one place

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Sounds reasonable to me.  It may also be worth uncommenting the MOZ_LOGGING ifdefs in CSFLog.h, which would make the code identical from the ImgLib stuff I cloned that from. I'm not sure what the consequences of that are, and I needed something which logged quickly, which is why I left those commented out for now.

Comment 4

[Ethan Hugg [:ehugg]]

Attachment: Ensure CSFLog.h comes before includes of prlog.h

Comment 5

[Ethan Hugg [:ehugg]]

Comment on attachment 714623 [details] [diff] [review]
Ensure CSFLog.h comes before includes of prlog.h


If this try and some testing go well, then I'll put it up for review

https://tbpl.mozilla.org/?tree=Try&rev=f323ca9be746

Comment 6

[Ethan Hugg [:ehugg]]

Attachment: Ensure CSFLog.h comes before includes of prlog.h

Comment 7

[Ethan Hugg [:ehugg]]

Comment on attachment 714687 [details] [diff] [review]
Ensure CSFLog.h comes before includes of prlog.h


OK, apparently there is no such thing as PR_LOG_INFO, trying again.

https://tbpl.mozilla.org/?tree=Try&rev=5effc5e0fa90

Comment 8

[Ethan Hugg [:ehugg]]

Running a try to check build for all targets:

https://tbpl.mozilla.org/?tree=Try&rev=4c5a925ec528

Comment 9

[Ethan Hugg [:ehugg]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4fcce10347ab

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Ensure CSFLog.h comes before includes of prlog.h (alder version)

Same patch, but rebased against alder for landing there.

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Landed on alder:

https://hg.mozilla.org/projects/alder/rev/f402f1948247

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4fcce10347ab