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 trc1log wrapped logger #128

Merged
merged 4 commits into from
Mar 26, 2021
Merged

Add trc1log wrapped logger #128

merged 4 commits into from
Mar 26, 2021

Conversation

advayakrishna
Copy link
Contributor

@advayakrishna advayakrishna commented Mar 24, 2021

Before this PR

This change adds a wrapped logging implementation for trc1logs.

As part of this change, the trc1 logger now adds span fields using a param instead of a custom marshaller so that it can be nested at different levels in the original vs. wrapped logging implementations.

After this PR

==COMMIT_MSG==
Add trc1log wrapped logger
==COMMIT_MSG==

Possible downsides?


This change is Reviewable

@changelog-app
Copy link

changelog-app bot commented Mar 24, 2021

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Add trc1log wrapped logger

Check the box to generate changelog(s)

  • Generate changelog entry

Copy link
Contributor

@bmoylan bmoylan left a comment

Choose a reason for hiding this comment

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

Reviewed 11 of 15 files at r1, 5 of 5 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @advayakrishna)


wlog/trclog/trc1log/logger_default.go, line 95 at r2 (raw file):

	fields := make(map[string]interface{})
	fields[AnnotationValueKey] = value
	fields[AnnotationTimestampKey] = timeStamp.Round(time.Microsecond).UnixNano() / 1e3

nit: divide by time.Microsecond instead of 1e3

Copy link
Contributor

@bmoylan bmoylan left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r3.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @advayakrishna)

@advayakrishna advayakrishna merged commit c8ffecf into develop Mar 26, 2021
@bmoylan bmoylan deleted the ak/wrapped-trc1logger branch March 26, 2021 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants