Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve logging with assertions #1196

Closed
gingerwizard opened this issue Feb 24, 2021 · 0 comments · Fixed by #1200
Closed

Improve logging with assertions #1196

gingerwizard opened this issue Feb 24, 2021 · 0 comments · Fixed by #1200
Labels
enhancement Improves the status quo

Comments

@gingerwizard
Copy link

Currently logging with assertions only indicates the task name failing. In the case of composite operations this makes it quite hard to identify which query actually failed the assertion.

I think this is small change on the Asserting Runner e.g.


    async def __call__(self, *args):
        params = args[1]
        return_value = await self.delegate(*args)
        if AssertingRunner.assertions_enabled and "assertions" in params:
            if isinstance(return_value, dict):
                for assertion in params["assertions"]:
                    try:
                        self.check_assertion(assertion, return_value)
                    except exceptions.RallyTaskAssertionError as e:
                        self.logger.info(f"Assertion failed on: {[params.get('name')]}")
                        raise e
            else:
                self.logger.debug("Skipping assertion check as [%s] does not return a dict.", repr(self.delegate))
        return return_value

The AssertingRunner has no visibility of the child runner and we dont have any guarantee of the params in args. The name should exist however and would be sufficient if the user names their searches uniquely within a composite - which should be good practice.

@gingerwizard gingerwizard added the enhancement Improves the status quo label Feb 24, 2021
danielmitterdorfer added a commit to danielmitterdorfer/rally that referenced this issue Feb 24, 2021
With this commit we attempt to extract an operation's `name` property
and if present, we include it in the assertion message. This is mostly
useful for operations inside a composite, as we lack context there and
the error message only indicates an error "somewhere" in the composite
but not exactly in which specific operation. With this commit, it is
possible to get that information.

Closes elastic#1196
danielmitterdorfer added a commit that referenced this issue Mar 1, 2021
With this commit we attempt to extract an operation's `name` property
and if present, we include it in the assertion message. This is mostly
useful for operations inside a composite, as we lack context there and
the error message only indicates an error "somewhere" in the composite
but not exactly in which specific operation. With this commit, it is
possible to get that information.

Closes #1196
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves the status quo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant