Skip to content

Fix kubernetes attach long lines

Georgi N. Georgiev requested to merge fix_kubernetes_attach_long_lines into master

What does this MR do?

This MR long log lines. The comment above splitLinesStartingWithDateWithMaxBufferSize explains in great detail.

@steveazz I would like to get your input on this. It's not a particularly pretty solution, but then again the problems it handles aren't either. Other, simpler options result in incorrect output:

  • Only concatenating lines while not handling the timestamps in the middle of the lines will output a lot of timestamps in the logs which will create noise.
  • Not using --timestamps isn't an option since we need them to differentiate between lines when reattaching. Otherwise, we will end up showing logs multiple times when reattaching to the log stream which might be confusing.

If you get a better idea how to handle this in a simpler manner I am all 👂.

There's a few more minor things I would like to finish, but wanted to get your eyes on it first:

  • More tests
  • Handle a line larger than maxLineBufferSize
  • Handle the TODO left in the splitLinesStartingWithDateWithMaxBufferSize function.

@lraykov As someone with particular Docker experience, maybe you could take a look? Maybe you would have some unique insights 🙇

Why was this MR needed?

Are there points in the code the reviewer needs to double check?

You could use the following configuration to test this on Gitlab.com:

build:
    script:
        - |
            for i in `seq 10 1000 100000`
            do
                echo $i
                for j in `seq $i`
                do
                    echo -n .
                done
                echo
            done

I also have the following simple node.js script which checks for the correct size of the log lines in the Gitlab.com log

const readlines = require('n-readlines');

const liner = new readlines('/Users/georgin.georgiev/Downloads/job.txt');

let i = -1;
let bytes = 0;
while (true) {
    i++;
    let line = liner.next().toString();
    if (i % 2 === 0) {
        bytes = parseInt(line.trim());
    } else {
        actualBytes = line.length;
        if (actualBytes !== bytes) {
            throw `expected ${bytes}, got ${actualBytes}`;
        }
    }
}

Just make sure to remove all job output before

10
..........

after downloading the job log. If the log is correct you should get the output:

expected 92010, got 4813

Since this is where the job output limit of 4MB is reached.

An easy way to see a visual representation of the logs is by doing kubectl logs -c build -f --timestamps <POD> when running the script above.

Does this MR meet the acceptance criteria?

  • Documentation created/updated
  • Added tests for this feature/bug
  • In case of conflicts with master - branch was rebased

What are the relevant issue numbers?

Closes #24928 (closed)

Edited by Georgi N. Georgiev

Merge request reports

Loading