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

Add logging of mappings in BaseLogger #3295

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

nowtryz
Copy link

@nowtryz nowtryz commented Oct 17, 2024

Fixes #3294

Description:
Adds support for Mapping metrics in loggers

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@github-actions github-actions bot added the module: handlers Core Handlers module label Oct 17, 2024
@nowtryz
Copy link
Author

nowtryz commented Oct 17, 2024

Before testing this, I would like to be sure the feature is of interest.

@nowtryz
Copy link
Author

nowtryz commented Oct 22, 2024

@vfdev-5 is it ok for you like that? If so, I'll fix the linting and adapt the tests

@nowtryz nowtryz requested a review from vfdev-5 October 22, 2024 19:59
Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nowtryz , this is going in a good direction, let's try to improve a bit more the code.

return metrics

@classmethod
def _compute_tags(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can make this just a private helper method in this submodule, not necessarily a classmethod.

Comment on lines +169 to +172
concat_tf: Callable[..., Union[str, Tuple[str, ...]]],
log_text: bool, node: Iterable[Tuple[str, Any]],
parent_tag: Union[str, Tuple[str, ...]],
dest_dict: Dict[Any, Union[str, float, numbers.Number]]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not keen of passing dest_dict as the argument, but maybe we can't do anything better here...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: handlers Core Handlers module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add the logging of dict metrics
2 participants