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

Stop printing Protobuf message and its metadata in logs #52

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

jeyhun
Copy link
Contributor

@jeyhun jeyhun commented Jul 27, 2023

Added String() method to ProtoStoreMsg, which will print out only ID, org. ID, instance ID, revision, and table name.


Before:

Unable to locate record [record=&{Id:P4 Msg:[] ParentId: Revision:0 OrgId:Pepsi InstanceId:Americas CreatedAt:0001-01-01 00:00:00 +0000 UTC UpdatedAt:0001-01-01 00:00:00 +0000 UTC DeletedAt:{Time:0001-01-01 00:00:00 +0000 UTC Valid:false} XTableName:processor}, dbName=testprotostoreindbcrud]
	record not found

After:

Unable to locate record [record={"id":"P4","revision":0,"org_id":"Pepsi","instance_id":"Americas","x_table_name":"processor"}, dbName=testprotostoreindbcrud]
	record not found

This resolves issue #42

@codecov-commenter
Copy link

Codecov Report

Merging #52 (a118699) into main (9b232f3) will decrease coverage by 0.10%.
The diff coverage is 33.33%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #52      +/-   ##
==========================================
- Coverage   70.56%   70.47%   -0.10%     
==========================================
  Files          12       12              
  Lines        1410     1419       +9     
==========================================
+ Hits          995     1000       +5     
- Misses        328      329       +1     
- Partials       87       90       +3     
Flag Coverage Δ
unit-tests 70.47% <33.33%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
pkg/protostore/protostore.go 68.28% <33.33%> (-0.45%) ⬇️

@jeyhun jeyhun force-pushed the ProtoStoreMsg-string-method branch 2 times, most recently from 7a32f03 to ca5a03a Compare July 27, 2023 19:26
@jeyhun jeyhun changed the title Stop printing Protobuf message as byte array and its metadata in logs Stop printing Protobuf message and its metadata in logs Jul 27, 2023
@jeyhun jeyhun marked this pull request as ready for review July 27, 2023 19:32
@jeyhun jeyhun self-assigned this Jul 27, 2023
@krishnamiriyala
Copy link
Contributor

@jeyhun If it is used in logging add sample log for reference

Added String() method to ProtoStoreMsg, which will print out only ID, org. ID, instance ID, revision, and table name.
@jeyhun jeyhun force-pushed the ProtoStoreMsg-string-method branch from ca5a03a to ba049e5 Compare July 27, 2023 21:55
@jeyhun
Copy link
Contributor Author

jeyhun commented Jul 27, 2023

@jeyhun If it is used in logging add sample log for reference

I've changed the PR's description to include a string version of an error generated by ProtoStore, which will include the JSON.
I haven't made the consumers of multi-tenant-persistence-for-saas use this change, so can't include consumers' logs.

Copy link
Contributor

@krishnamiriyala krishnamiriyala left a comment

Choose a reason for hiding this comment

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

Lets fix the example for readability separately.

@krishnamiriyala krishnamiriyala merged commit 145bb5a into main Jul 28, 2023
2 checks passed
@krishnamiriyala krishnamiriyala deleted the ProtoStoreMsg-string-method branch September 19, 2023 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants