grafana / xk6-browser

k6 extension that adds support for browser automation and end-to-end web testing via the Chrome Devtools Protocol
https://grafana.com/docs/k6/latest/javascript-api/k6-experimental/browser/
GNU Affero General Public License v3.0
337 stars 42 forks source link

Implement tracking of `data_sent`/`data_received` metrics #40

Closed imiric closed 2 years ago

imiric commented 2 years ago

Similarly how this is done for native k6 requests/responses, the extension should emit data_sent/data_received metrics as well.

inancgumus commented 2 years ago

@imiric

Can it be done (roughly) with something like this?

For data received:

func (c *Connection) recvLoop() {
    for {
        _, buf, err := c.conn.ReadMessage()
        if err != nil {
            c.handleIOError(err)
            return
        }

        c.logger.Debugf("cdp:recv", "<- %s", buf)

        // THIS ?
        //-----------------------------------------------
        state := k6lib.GetState(c.ctx)
        k6stats.PushIfNotDone(c.ctx, state.Samples, k6stats.ConnectedSamples{
            Samples: []k6stats.Sample{
                {
                    Metric: k6metrics.DataReceived,
                    Value:  float64(len(buf)),
                    Time:   time.Now(),
                },
            },
        })
        //-----------------------------------------------
                 ...

For data sent:

func (c *Connection) sendLoop() {
    for {
        select {
        case msg := <-c.sendCh:
            c.encoder = jwriter.Writer{}
            msg.MarshalEasyJSON(&c.encoder)
            if err := c.encoder.Error; err != nil {
                select {
                case c.errorCh <- err:
                case <-c.done:
                    return
                }
            }

            buf, _ := c.encoder.BuildBytes()
            c.logger.Debugf("cdp:send", "-> %s", buf)
            writer, err := c.conn.NextWriter(websocket.TextMessage)
            if err != nil {
                c.handleIOError(err)
                return
            }
            if _, err := writer.Write(buf); err != nil {
                c.handleIOError(err)
                return
            }
            if err := writer.Close(); err != nil {
                c.handleIOError(err)
                return
            }
            // THIS ?
            //-----------------------------------------------
            state := k6lib.GetState(c.ctx)
            k6stats.PushIfNotDone(c.ctx, state.Samples, k6stats.ConnectedSamples{
                Samples: []k6stats.Sample{
                    {
                        Metric: k6metrics.DataSent,
                        Value:  float64(len(buf)),
                        Time:   time.Now(),
                    },
                },
            })
            //-----------------------------------------------
                         ...

Result:

$ xk6 run examples/querying.js
     ....
     data_received....................: 83 kB 40 kB/s
     data_sent...........................: 36 kB 17 kB/s
     ...
imiric commented 2 years ago

@inancgumus Seems about right to me. Maybe get the State outside of the loop and check if it's not nil, but LGTM otherwise. Create a PR?

mstoykov commented 2 years ago

This won't work with the cloud output as it has special handling for data_sent/data_received and unfortunately if you just emit netext.Trail that is also used to report that an iteration is finished ... but you should probably double-check that as my info can be off (especially on the second part)

robingustafsson commented 2 years ago

I think the data_sent and data_received should track the HTTP traffic generated by the browser not the CDP traffic between the browser and xk6-browser. That's not very relevant I'd say ;)

inancgumus commented 2 years ago

Thanks, @robingustafsson; I'm still trying to grasp the parts in the extension :)

Where is the best place to track the browser's HTTP traffic? Does it exist in the extension somewhere? Where should we implement it best? Any ideas?

inancgumus commented 2 years ago

This won't work with the cloud output as it has special handling for data_sent/data_received and unfortunately if you just emit netext.Trail that is also used to report that an iteration is finished ... but you should probably double-check that as my info can be off (especially on the second part)

@MStoykov I'm confused 😕 Could you point me to a correct implementation? Did you implement it in another extension or code where I can look at?

robingustafsson commented 2 years ago

@inancgumus Have a look here:

Just make sure that the request/response bodies have been fetched from the browser (or find another way to get the body size, eg. looking at content-length header or similar).

mstoykov commented 2 years ago

Could you point me to a correct implementation?

I don't think there is such one as the current way these metrics are handled (both in k6 and the backend) don't really lend themselves to an extension emit those unless they actually use the Dialer in the lib.State, which will do this for you.

@dgzlopes already hit this in the prometheus remote write client(which is how i know there are problems), maybe he can say what the problems were with just emitting netTrail as I see that he did for a while

inancgumus commented 2 years ago

@MStoykov Thanks! Could you explain what is NetTrail? And what does it do, why is it necessary? Why do we need a Dialer? 😞

dgzlopes commented 2 years ago

The problem I had was that some metrics (DataSent) would work well locally, but not while using the Cloud output.

I asked for guidance on Slack. Here is the thread: https://loadimpact.slack.com/archives/CBAF22F2R/p1633082640093300

dgzlopes commented 2 years ago

But... The latest version of the Prometheus extension doesn't use this method. I dropped it in favor of using the helpers available in httpext to make the requests. So... I didn't have enough time to validate/find the problems that Mihail explains about iterations.