status-im / status-protocol-go

Status Protocol implementation in Go
Mozilla Public License 2.0
0 stars 1 forks source link

Unify logging in tests #38

Closed adambabik closed 5 years ago

adambabik commented 5 years ago

I noticed that our logging strategy in tests, in terms of selected logger and logs format, is not unified. This change fixes this.

There is still one problem when byte slices are not hex-encoded when displayed. It's a situation where the logger is used this way:

logger := app.logger.WithFields("hash", zap.Binary("messageID", m.ID()))
logger.Debug("some message")

In this case, messageID field will not be hex-encoded.

This will work fine:

logger.Debug("some message", zap.Binary("messageID", m.ID()))

I described the problem in https://github.com/uber-go/zap/issues/730.

pedropombeiro commented 5 years ago

There is still one problem when byte slices are not hex-encoded when displayed. It's a situation where the logger is used this way:

logger := app.logger.WithFields("hash", zap.Binary("messageID", m.ID()))
logger.Debug("some message")

In this case, messageID field will not be hex-encoded.

This will work fine:

logger.Debug("some message", zap.Binary("messageID", m.ID()))

@adambabik Isn't it happening the other way around? I tested this branch, and see this:

2019-07-30T10:52:27.342+0200    DEBUG   sharedsecret/sharedsecret.go:53 saving a shared key     {"user": "bob", "SharedSecret": {"site": "generate", "their-public-key": "BOpaFoVJ/nSvfTE48iiqyZdNz5E1PXPWbdS3DrV3HQ3vUQUxoIF0l8zWopHL+a7upLCKSuU8Frm75argTLlKdVc=", "installation-id": "bob1"}}
2019-07-30T10:52:27.342+0200    DEBUG   sharedsecret/sharedsecret.go:99 no shared secret for installation       {"user": "bob", "SharedSecret": {"site": "Agreed", "installation-id": "alice1"}}
2019-07-30T10:52:27.342+0200    DEBUG   encryption/protocol.go:257      shared secret agreement {"user": "bob", "Protocol": {"site": "BuildDirectMessage", "public-key": "0x04ea5a168549fe74af7d3138f228aac9974dcf91353d73d66dd4b70eb5771d0def510531a0817497ccd6a291cbf9aeeea4b08a4ae53c16b9bbe5aae04cb94a7557", "has-shared-secret": true, "agreed": false}}

Looking at the code, SharedSecret.generate is using:

    logger.Debug(
        "saving a shared key",
        zap.Binary("their-public-key", crypto.FromECDSAPub(theirPublicKey)),
        zap.String("installation-id", installationID),
    )

and BuildDirectMessage (encoded with 0x) is using:

    logger := p.logger.With(
        zap.String("site", "BuildDirectMessage"),
        zap.Binary("public-key", crypto.FromECDSAPub(publicKey)),
    )
adambabik commented 5 years ago

@PombeirP oh right. I mixed the order of the snippets. Thanks!