ObserveRTC / client-monitor-js

Javascript library to monitor browser-side webrtc app
https://observertc.org/docs/api/client-monitor-js-v2/
Apache License 2.0
31 stars 3 forks source link

Missing REST sender configuration #8

Closed ValarMarkhulis closed 1 year ago

ValarMarkhulis commented 1 year ago

According to the Observer's readme file it is possible to configure the server so it both accepts samples over REST and Websockets.

But the example configuration given in the client-monitor-js does not seem to allow or have the option to set a REST endpoint, but only a list of WebSocket URLs? Is this correctly understood or am I missing something obvious?

    /**
     * Configure the sender component.
     */
    sender: {
        /**
         * Configure the format used to transport samples or receieve 
         * feedback from the server.
         * 
         * Possible values: json, protobuf
         * 
         * DEFAULT: json
         * 
         */
        format: "json",
         /**
         * Websocket configuration to transport the samples
         */
        websocket: {
            /**
             * Target urls in a priority order. If the Websocket has not succeeded for the first,
             * it tries with the second. If no more url left the connection is failed
             * 
             */
            urls: ["ws://localhost:7080/samples/myServiceId/myMediaUnitId"],
            /**
             * The maximum number of retries to connect to a server before,
             * tha connection failed is stated.
             * 
             * DEFAULT: 3
             */
            maxRetries: 1,
        }
    },
ValarMarkhulis commented 1 year ago

I can see that the sfu-monitor does seem to have the REST endpoint option. But due to the comments not being consistent with the code, I doubt that it works.

    sender: {
        /**
         * Configure the codec used to transport samples or receieve
         * feedback from the server.
         *
         * Possible values: json, protobuf
         *
         * DEFAULT: json
         *
         */
        format: "json",
        /**
         * Websocket configuration to transport the samples
         */
        websocket: {
            /**
             * The target urls the websocket is opened for
             */
            urls: ["ws://localhost:7080/samples/myServiceId/myMediaUnitId"],
            /**
             * The maximum number of try to connect to the server
             *
             * DEFAULT: 3
             */
            maxRetry: 1,
            /**
             * An optional field for additional socket option from ws package
             */
            socketOptions: {

            },
        },
        /**
         * Configuration to setup a REST api transport method for the samples.
         */
        rest: {
            /**
             * The target url the websocket is opened for
             */
            urls: ["http://localhost:7080/rest/samples/myServiceId/myMediaUnitId"];
            /**
             * The maximum number of try to connect to the server
             *
             * DEFAULT: 3
             */
            maxRetry: 1,
        },
    }
};
balazskreith commented 1 year ago

Hi @ValarMarkhulis ,

Currently in client-monitor-js only the websocket transport is added, however if it is needed we can add the rest as well

For the client-side Its mainly a copy-paste from the sfu-monitor-js. As for the server side, and this is the reason I have not spend time with it yet, need to look for possible CORS issues in this case.

But Fair point. REST was added to the observer mainly for the sfu-monitor-js, as in many cases the sampling and sending time there is over 60s.

ValarMarkhulis commented 1 year ago

Hi @balazskreith

Thank you for the response and the work you do on the project. Awesome to see an open-source alternative to the paid WebRTC monitoring services like callstats.io and testrtc.com. I will try to look at your suggestion and get back to you if I have any problems.

balazskreith commented 1 year ago

Thanks.

REST for sfu-monitor-js should work, at least that's the method I used in full-stack to send samples from mediasoup.

If you want to make a PR for REST client-monitor-js, shout out here if you have any further questions or issues. If you run some problems with observer CORS to accept REST requests from the client-monitor, then open an issue at observer.

balazskreith commented 1 year ago

Closing this issue as from 2.0.0 transport capability is pushed out from client-monitor library. If any specific or custom transport is needed the application uses client-monitor should do it. Simply subscribe to clientMonitor.on('send', ({ samples }) => {}) event in the app and do the rest