scylladb / argus

Apache License 2.0
4 stars 11 forks source link

Include test name in Run Details section #375

Closed soyacz closed 3 months ago

soyacz commented 5 months ago

When looking at performance results we usually have 3 runs for the same build number - and it's not easy to guess which test it is (write, read or mixed).

Idea is to add test name to run details section so we can distinguish them.

soyacz commented 3 months ago

It's not trivial - we don't send this information and adding it requires adding feature to client and SCT. It's not hard either.

@fruch now when I implemented results in Argus, we can see the workload based on result tables. Do you think it's still worth the effort?

fruch commented 3 months ago

It's not trivial - we don't send this information and adding it requires adding feature to client and SCT. It's not hard either.

@fruch now when I implemented results in Argus, we can see the workload based on result tables. Do you think it's still worth the effort?

it's helpful not just for perf tests,

but how come we don't have the info ? doesn't argus get the full SCT configuration ? i would expect the test name to be there

fruch commented 3 months ago

SCT pass it down:

            self.test_config.argus_client().submit_sct_run(
                job_name=get_job_name(),
                job_url=get_job_url(),
                started_by=get_username(),
                commit_id=git_status.get('branch.oid', get_git_commit_id()),
                origin_url=git_status.get('upstream.url'),
                branch_name=git_status.get('branch.upstream'),
                sct_config=self.params.dict(),
            )

what argus is saving out of it, if all of it, or just parts of it, I don't know

soyacz commented 3 months ago

SCT pass it down:

            self.test_config.argus_client().submit_sct_run(
                job_name=get_job_name(),
                job_url=get_job_url(),
                started_by=get_username(),
                commit_id=git_status.get('branch.oid', get_git_commit_id()),
                origin_url=git_status.get('upstream.url'),
                branch_name=git_status.get('branch.upstream'),
                sct_config=self.params.dict(),
            )

what argus is saving out of it, if all of it, or just parts of it, I don't know

I don't know which param you refer to - we need something that is a result of a :

        test_name = self._testMethodName
        class_name = self.__class__.__name__
        full_test_name = f"{class_name}.{test_name}"

(executed in a context of ClusterTester)

fruch commented 3 months ago

Right I've forgot it's not in sct_config, like almost everything else:

so let's introduce it into sct_config, and pass it down same as SCT_CONFIG_FILES and SCT_CLUSTER_BACKEND below ?

@cli.command('run-test', help="Run SCT test using unittest")
@click.argument('argv')
@click.option('-b', '--backend', type=click.Choice(SCTConfiguration.available_backends), help="Backend to use")
@click.option('-c', '--config', multiple=True, type=click.Path(exists=True), help="Test config .yaml to use, can have multiple of those")
@click.option('-l', '--logdir', help="Directory to use for logs")
def run_test(argv, backend, config, logdir):
    if config:
        os.environ['SCT_CONFIG_FILES'] = str(list(config))
    if backend:
        os.environ['SCT_CLUSTER_BACKEND'] = backend

    if logdir:
        os.environ['_SCT_LOGDIR'] = logdir

    logfile = os.path.join(get_test_config().logdir(), 'output.log')
    sys.stdout = OutputLogger(logfile, sys.stdout)
    sys.stderr = OutputLogger(logfile, sys.stderr)

    unittest.main(module=None, argv=['python -m unittest', argv],
                  failfast=False, buffer=False, catchbreak=True, testLoader=SctLoader())

it's a small change in SCT, and you'll start having those values on argus end...

soyacz commented 3 months ago

Right I've forgot it's not in sct_config, like almost everything else:

so let's introduce it into sct_config, and pass it down same as SCT_CONFIG_FILES and SCT_CLUSTER_BACKEND below ?

@cli.command('run-test', help="Run SCT test using unittest")
@click.argument('argv')
@click.option('-b', '--backend', type=click.Choice(SCTConfiguration.available_backends), help="Backend to use")
@click.option('-c', '--config', multiple=True, type=click.Path(exists=True), help="Test config .yaml to use, can have multiple of those")
@click.option('-l', '--logdir', help="Directory to use for logs")
def run_test(argv, backend, config, logdir):
    if config:
        os.environ['SCT_CONFIG_FILES'] = str(list(config))
    if backend:
        os.environ['SCT_CLUSTER_BACKEND'] = backend

    if logdir:
        os.environ['_SCT_LOGDIR'] = logdir

    logfile = os.path.join(get_test_config().logdir(), 'output.log')
    sys.stdout = OutputLogger(logfile, sys.stdout)
    sys.stderr = OutputLogger(logfile, sys.stderr)

    unittest.main(module=None, argv=['python -m unittest', argv],
                  failfast=False, buffer=False, catchbreak=True, testLoader=SctLoader())

it's a small change in SCT, and you'll start having those values on argus end...

Still we don't support sending config itself - just config file names.

fruch commented 3 months ago

@soyacz

Here Argus can get sct_config as a dict: https://github.com/scylladb/argus/blob/0a9fd06d6ee15e40eed69ed4ef07310948c923a3/argus/client/sct/client.py#L34

It's the full SCT configuration, every single configuration is pass to the Argus client

soyacz commented 3 months ago

@soyacz

Here Argus can get sct_config as a dict:

https://github.com/scylladb/argus/blob/0a9fd06d6ee15e40eed69ed4ef07310948c923a3/argus/client/sct/client.py#L34

It's the full SCT configuration, every single configuration is pass to the Argus client

Now I see it. It will simplify the effort (no need to client update). I'll do it today.