[jitsi-dev] [jitsi/jitsi-videobridge] Adds statistics about total number of failed/succeeded conferences/channels. (#286)


#1

You can view, comment on, or merge this pull request online at:

  https://github.com/jitsi/jitsi-videobridge/pull/286

-- Commit Summary --

  * Add 'lastTransportActivityTime' to Channel
  * refactor(rtp): Fine grained channel activity logging.
  * refactor(conference): Exposes the includeInStatistics field.
  * feat(stats): Exposes total/failed channel/conference.
  * refactor(channel): Merges touchTransport/Payload into touch.

-- File Changes --

    M src/main/java/org/jitsi/videobridge/Channel.java (84)
    M src/main/java/org/jitsi/videobridge/Conference.java (110)
    M src/main/java/org/jitsi/videobridge/IceUdpTransportManager.java (2)
    M src/main/java/org/jitsi/videobridge/RtpChannel.java (28)
    M src/main/java/org/jitsi/videobridge/Videobridge.java (54)
    M src/main/java/org/jitsi/videobridge/stats/VideobridgeStatistics.java (66)

-- Patch Links --

https://github.com/jitsi/jitsi-videobridge/pull/286.patch
https://github.com/jitsi/jitsi-videobridge/pull/286.diff

···

--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286


#2

@gpolitis pushed 1 commit.

bdda6c1 [docs] Adds/Updates comments about stats.

···


You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/340e6a8bce61d727e3bd4d671a07a60c3c945627..bdda6c14afa5381f6a216b8b7c5a7aef60fb81a6


#3

@@ -117,6 +117,16 @@
         = new MonotonicAtomicLong();

     /**
+ * The time in milliseconds of the last transport related activity to this

The doc needs an update

···

--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/0c7355f740ce425337092252bd598520efb4df33#r74858214


#4

@@ -127,6 +127,14 @@
         = new MonotonicAtomicLong();

     /**
+ * The time in milliseconds of the last payload related activity to this
+ * <tt>Channel</tt>. Currently this means when for the last time there were
+ * any RTP packets seen for this channel.

Can we replace "seen" with "received"?

···


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/0c7355f740ce425337092252bd598520efb4df33..51d267b3c3023d4fbd1c04d299a785e156c313be#r74858358


#5

@@ -1193,7 +1193,7 @@ protected void maybeStartStream()
         }

         // It seems this Channel is still active.
- touch(true /* triggered by transport related event */);
+ touch(ActivityType.TRANSPORT /* transport connected */);

Why don't we move that to transportConnected()?

···


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/0c7355f740ce425337092252bd598520efb4df33..51d267b3c3023d4fbd1c04d299a785e156c313be#r74858676


#6

+ videobridgeStatistics.totalFailedConferences.incrementAndGet();
+ }
+
+ if (logger.isInfoEnabled())
+ {
+
+ int[] metrics
+ = videobridge.getConferenceChannelAndStreamCount();
+
+ logger.info(
+ "Expired conference id=" + getID()
+ + ", conferenceCount="
+ + metrics[0]
+ + ", channelCount="
+ + metrics[1]
+ + ", streamCount="

Can we revert to the original "video streams" here? It has special meaning (I agree that it isn't accurate) and I'd rather just use the term that we already use.

···


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/ec19e6e13d9822b7a8ffcb46ca761edc7aecf410..73aab6db869f9e1bcfb1c06e1b2742ab1d9eb965#r74859396


#7

@@ -1560,4 +1576,42 @@ private int getContentStreamCount(Content content, int contentChannelCount)
         }
         return contentStreamCount;
     }
+
+ /**
+ * Basic statistics about the videobridge.
+ */
+ public static class Statistics
+ {
+ /**

Maybe add docs?

···


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/ec19e6e13d9822b7a8ffcb46ca761edc7aecf410..73aab6db869f9e1bcfb1c06e1b2742ab1d9eb965#r74859632


#8

                 break;
- case PAYLOAD:
- touchPayload();
+ default:
+ lastActivityTime.increase(now);

Maybe merge the three lastActivityTime.increase(now) calls outside the "switch"?

···


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/73aab6db869f9e1bcfb1c06e1b2742ab1d9eb965..340e6a8bce61d727e3bd4d671a07a60c3c945627#r74859901


#9

Can we add the new fields to docs/using_statistics.md? Or maybe wait until #284 is merged to avoid dealing with conflicts?

···

--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286#issuecomment-239969875


#10

@@ -117,6 +117,16 @@
         = new MonotonicAtomicLong();

     /**
+ * The time in milliseconds of the last transport related activity to this

@bgrozev could you please be a little more precise?

···

--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/0c7355f740ce425337092252bd598520efb4df33#r74954701


#11

@gpolitis pushed 2 commits.

76f4494 docs: Changes the wording in a couple of places.
65a4e01 refactor: Touches the channel on transport connected.

···


You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/bdda6c14afa5381f6a216b8b7c5a7aef60fb81a6..65a4e01c0aabbebc19584697853cb23f881db67f


#12

@gpolitis pushed 1 commit.

928df1e logging: Partially restores the formatting of a debug log.

···


You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/65a4e01c0aabbebc19584697853cb23f881db67f..928df1e7a36c4a60f28f2f331cfa441cab3e7421


#13

@gpolitis pushed 1 commit.

b7fd0b2 docs: Adds JavaDoc comments to the Videobridge.Statistics class.

···


You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/928df1e7a36c4a60f28f2f331cfa441cab3e7421..b7fd0b26eb447d9433ad5bfebd6feea1ee488438


#14

@gpolitis pushed 1 commit.

3e818e3 refactor: changes the syntax of a switch statement.

···


You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/b7fd0b26eb447d9433ad5bfebd6feea1ee488438..3e818e3c03bbcd3424784efcd974b3614df212e1


#15

I think we would like to have this merged as soon as possible to start gathering data. Would you mind merging this if I push a commit to https://github.com/jitsi/jitsi-videobridge/pull/284 to update the statistics documentation?

···

--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286#issuecomment-240147208


#16

@@ -117,6 +117,16 @@
         = new MonotonicAtomicLong();

     /**
+ * The time in milliseconds of the last transport related activity to this

Are you talking about the change in the switch statement?

···

--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/0c7355f740ce425337092252bd598520efb4df33#r74980670


#17

@@ -117,6 +117,16 @@
         = new MonotonicAtomicLong();

     /**
+ * The time in milliseconds of the last transport related activity to this

No, I mis-read the code.

···

--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286/files/0c7355f740ce425337092252bd598520efb4df33#r75003781


#18

I think we would like to have this merged as soon as possible to start gathering data. Would you mind merging this if I push a commit to #284 to update the statistics documentation?

Yeah, sure

···

--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286#issuecomment-240213251


#19

Merged #286.

···

--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/286#event-757590909


#20

After the latest commits the "transport" activity does not include rtp, but
the doc states that it does. I may be missing something, will check again
when I have a computer.

Boris

···

On Tuesday, August 16, 2016, George Politis <notifications@github.com> wrote:

In src/main/java/org/jitsi/videobridge/Channel.java
<https://github.com/jitsi/jitsi-videobridge/pull/286#discussion_r74954701>
:

> @@ -117,6 +117,16 @@
> = new MonotonicAtomicLong();
>
> /**
> + * The time in milliseconds of the last transport related activity to this

@bgrozev <https://github.com/bgrozev> could you please be a little more
precise?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<https://github.com/jitsi/jitsi-videobridge/pull/286/files/0c7355f740ce425337092252bd598520efb4df33#r74954701>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AER2Y5vtBisLPiFO-1i11pfbaPooEq9Lks5qgdMAgaJpZM4Jk4hN>
.