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.

Sign up for GitHub

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

Jump to bottom

Bug fix for data race #354

Merged
merged 11 commits into from
Nov 19, 2020
Merged

Bug fix for data race #354

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bb428b9
comment some tests and add a lock
yashrsharma44 Oct 6, 2020
3105f3c
Added the latest version of grpc middleware
yashrsharma44 Oct 6, 2020
2b54459
Added the new version of middleware commit for importing the new API …
yashrsharma44 Oct 6, 2020
5eff929
more changes
yashrsharma44 Oct 8, 2020
1b1a25d
Added some locks and checks
yashrsharma44 Oct 8, 2020
c4b4319
Merge branch 'merge-log' into bug-fix
yashrsharma44 Oct 9, 2020
89a233f
added some changes as suggested by reviewer
yashrsharma44 Oct 12, 2020
f6aaf5e
Extra changes
yashrsharma44 Oct 14, 2020
e3642d0
Renamed the baseMockLogger to sharedResults and added some more locks
yashrsharma44 Oct 14, 2020
52d5001
Merge branch 'shared-result' into bug-fix
yashrsharma44 Oct 14, 2020
d453e90
Renamed the sharedResults to mockStdOutput and sorted the stdoutput s…
yashrsharma44 Oct 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 21 additions & 21 deletions interceptors/logging/interceptors_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,7 @@ func (l LogLines) Less(i, j int) bool {
if l[i].msg != l[j].msg {
return l[i].msg < l[j].msg
}
//_ , aok = l[i].fields["grpc.response.content"]
//_ ,baok = l[i].fields["grpc.response.content"]
//if


// We want to sort by counter which in string, so we need to parse it.
a := testpb.PingResponse{}
b := testpb.PingResponse{}
Expand All @@ -88,47 +85,50 @@ func (l LogLines) Swap(i, j int) {
l[i], l[j] = l[j], l[i]
}

type baseMockLogger struct {
// Shared. It's slice on purpose to find potential duplicates.
type sharedResults struct {
// Contains a shared slice among all mockLoggers,
// for appending the Log results
lines []LogLine
m sync.Mutex
}

func (l *baseMockLogger) Add(line LogLine) {
func (l *sharedResults) Lines() []LogLine {
l.m.Lock()
defer l.m.Unlock()

l.lines = append(l.lines, line)
}
retLines := make([]LogLine, len(l.lines))
copy(retLines, l.lines)

func (l *baseMockLogger) Lines() []LogLine {
l.m.Lock()
defer l.m.Unlock()

return l.lines
return retLines
}

type mockLogger struct {
*baseMockLogger
*sharedResults
Copy link
Collaborator

Choose a reason for hiding this comment

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

I remember now. So this is essentially some kind of output (we can call it like this). Similar to os.StdErr. You push and it goes to some output. Normally it's shared so wanted to mimic this.

The problem might be that it's used concurrently so there is no way to guarantee order.

So we can either sort lines before comparing or remove sharedResult and keep references to all create loggers with With? Whatever works. Maybe if you name it output it will make more sense? (:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great, thanks for confirming what I understood!

So we can either sort lines before comparing or remove sharedResult and keep references to all create loggers with With? Whatever works. Maybe if you name it output it will make more sense? (:

I think, I would keep the original method of comparing the results, that sounds much more dependable. The references might create a bug-prone experience for comparing the results, so I would stick with the original method.

I have renamed the sharedResults into mockStdOutput, and have added some comments for how we are using it as a shared struct. Would be happy to add more comments if needed.

I have also sorted the output lines so that we have a definitive order for comparing the values.


fields logging.Fields
}

func (l *mockLogger) Log(lvl logging.Level, msg string) {
l.m.Lock()
defer l.m.Unlock()

line := LogLine{
lvl: lvl,
msg: msg,
fields: map[string]string{},
}

for i := 0; i < len(l.fields); i += 2 {
line.fields[l.fields[i]] = l.fields[i+1]
}
l.Add(line)
l.lines = append(l.lines, line)
}

func (l *mockLogger) With(fields ...string) logging.Logger {
// Append twice to copy slice, so we don't reuse array.
return &mockLogger{baseMockLogger: l.baseMockLogger, fields: append(append(logging.Fields{}, l.fields...), fields...)}
l.m.Lock()
defer l.m.Unlock()

return &mockLogger{sharedResults: l.sharedResults, fields: append(append(logging.Fields{}, l.fields...), fields...)}
}

type baseLoggingSuite struct {
Expand Down Expand Up @@ -161,7 +161,7 @@ func TestSuite(t *testing.T) {

s := &loggingClientServerSuite{
&baseLoggingSuite{
logger: &mockLogger{baseMockLogger: &baseMockLogger{}},
logger: &mockLogger{sharedResults: &sharedResults{}},
InterceptorTestSuite: &grpctesting.InterceptorTestSuite{
TestService: &grpctesting.TestPingService{T: t},
},
Expand Down Expand Up @@ -344,7 +344,7 @@ func TestCustomDurationSuite(t *testing.T) {

s := &loggingCustomDurationSuite{
baseLoggingSuite: &baseLoggingSuite{
logger: &mockLogger{baseMockLogger: &baseMockLogger{}},
logger: &mockLogger{sharedResults: &sharedResults{}},
InterceptorTestSuite: &grpctesting.InterceptorTestSuite{
TestService: &grpctesting.TestPingService{T: t},
},
Expand Down Expand Up @@ -466,7 +466,7 @@ func TestCustomDeciderSuite(t *testing.T) {

s := &loggingCustomDeciderSuite{
baseLoggingSuite: &baseLoggingSuite{
logger: &mockLogger{baseMockLogger: &baseMockLogger{}},
logger: &mockLogger{sharedResults: &sharedResults{}},
InterceptorTestSuite: &grpctesting.InterceptorTestSuite{
TestService: &grpctesting.TestPingService{T: t},
},
Expand Down
2 changes: 1 addition & 1 deletion interceptors/logging/payload_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestPayloadSuite(t *testing.T) {

s := &loggingPayloadSuite{
baseLoggingSuite: &baseLoggingSuite{
logger: &mockLogger{baseMockLogger: &baseMockLogger{}},
logger: &mockLogger{sharedResults: &sharedResults{}},
InterceptorTestSuite: &grpctesting.InterceptorTestSuite{
TestService: &grpctesting.TestPingService{T: t},
},
Expand Down
2 changes: 1 addition & 1 deletion providers/kit/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ go 1.14

require (
github.com/go-kit/kit v0.10.0
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891
google.golang.org/grpc v1.26.0
)
3 changes: 3 additions & 0 deletions providers/kit/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de
github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea h1:1Tk1IbruXbunEnaIZEFb+Hpv9BIZti3OxKwKn5wWyKk=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea/go.mod h1:GugMBs30ZSAkckqXEAIEGyYdDH6EgqowG8ppA3Zt+AY=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891 h1:2F7/en805byWQR92etfFjOqtRtWsUu09R7wm6LtlHEw=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891/go.mod h1:GhphxcdlaRyAuBSvo6rV71BvQcvB/vuX8ugCyybuS2k=
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk=
github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY=
github.com/hashicorp/consul/api v1.3.0/go.mod h1:MmDNSzIMUjNpY/mQ398R4bk2FnqQLoPndWW5VkKPlCE=
Expand Down Expand Up @@ -191,6 +193,7 @@ github.com/pierrec/lz4 v1.0.2-0.20190131084431-473cd7ce01a1/go.mod h1:3/3N9NVKO0
github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY=
github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/profile v1.2.1/go.mod h1:hJw3o1OdXxsrSjjVksARp5W95eeEaEfptyVZyv6JUPA=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
Expand Down
2 changes: 1 addition & 1 deletion providers/logrus/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/grpc-ecosystem/go-grpc-middleware/providers/logrus/v2
go 1.14

require (
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891
github.com/sirupsen/logrus v1.5.0
google.golang.org/grpc v1.19.0
)
3 changes: 3 additions & 0 deletions providers/logrus/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@ github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs
github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea h1:1Tk1IbruXbunEnaIZEFb+Hpv9BIZti3OxKwKn5wWyKk=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea/go.mod h1:GugMBs30ZSAkckqXEAIEGyYdDH6EgqowG8ppA3Zt+AY=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891 h1:2F7/en805byWQR92etfFjOqtRtWsUu09R7wm6LtlHEw=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891/go.mod h1:GhphxcdlaRyAuBSvo6rV71BvQcvB/vuX8ugCyybuS2k=
github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
github.com/konsorten/go-windows-terminal-sequences v1.0.1 h1:mweAR1A6xJ3oS2pRaGiHgQ4OO8tzTaLawm8vnODuwDk=
github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/sirupsen/logrus v1.5.0 h1:1N5EYkVAPEywqZRJd7cwnRtCb6xJx7NH3T3WUTF980Q=
Expand Down
2 changes: 1 addition & 1 deletion providers/zap/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/grpc-ecosystem/go-grpc-middleware/providers/zap/v2
go 1.14

require (
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891
go.uber.org/zap v1.15.0
google.golang.org/grpc v1.19.0
)
3 changes: 3 additions & 0 deletions providers/zap/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y
github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea h1:1Tk1IbruXbunEnaIZEFb+Hpv9BIZti3OxKwKn5wWyKk=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea/go.mod h1:GugMBs30ZSAkckqXEAIEGyYdDH6EgqowG8ppA3Zt+AY=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891 h1:2F7/en805byWQR92etfFjOqtRtWsUu09R7wm6LtlHEw=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891/go.mod h1:GhphxcdlaRyAuBSvo6rV71BvQcvB/vuX8ugCyybuS2k=
github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
Expand All @@ -26,6 +28,7 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
Expand Down
2 changes: 1 addition & 1 deletion providers/zerolog/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/grpc-ecosystem/go-grpc-middleware/providers/zerolog/v2
go 1.14

require (
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891
github.com/rs/zerolog v1.19.0
google.golang.org/grpc v1.19.0
)
3 changes: 3 additions & 0 deletions providers/zerolog/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@ github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs
github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea h1:1Tk1IbruXbunEnaIZEFb+Hpv9BIZti3OxKwKn5wWyKk=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-20200501113911-9a95f0fdbfea/go.mod h1:GugMBs30ZSAkckqXEAIEGyYdDH6EgqowG8ppA3Zt+AY=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891 h1:2F7/en805byWQR92etfFjOqtRtWsUu09R7wm6LtlHEw=
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20201002093600-73cf2ae9d891/go.mod h1:GhphxcdlaRyAuBSvo6rV71BvQcvB/vuX8ugCyybuS2k=
github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ=
Expand Down