feat(logger) added logger to be passed through the sagas #86
Conversation
vladshub
requested review from
avigailberger and
rhinof
and removed request for
avigailberger
gbus/bus.go
Outdated
| @@ -36,6 +37,7 @@ type DefaultBus struct { | |||
| amqpErrors chan *amqp.Error | |||
| amqpBlocks chan amqp.Blocking | |||
| Registrations []*Registration | |||
| logger logrus.FieldLogger | |||
There was a problem hiding this comment.
not critical but I think "log" would be read better
b.log().WithFields(...).Error(...)
vs
b.logger().WithFields(...).Error(...)
gbus/abstractions.go
Outdated
| @@ -185,6 +187,7 @@ type Invocation interface { | |||
| Bus() Messaging | |||
| Tx() *sql.Tx | |||
| Ctx() context.Context | |||
| Logger() logrus.FieldLogger | |||
There was a problem hiding this comment.
not sure whether we should expose the package specific implementation(logrus) in the interface used by handlers...what would be the migration path if we decide to move away from logrus in future releases?
gbus/saga/glue.go
Outdated
| @@ -190,7 +190,9 @@ func (imsm *Glue) invokeSagaInstance(instance *Instance, invocation gbus.Invocat | |||
| inboundMsg: message, | |||
| sagaID: instance.ID, | |||
| ctx: invocation.Ctx(), | |||
| invokingService: imsm.svcName} | |||
| invokingService: imsm.svcName, | |||
| logger: imsm.log().WithField("saga_id", instance.ID), | |||
There was a problem hiding this comment.
I would add the type of the saga and maybe the type/message fqn that invokes the saga instance as fields as well
|
Need to rebase this PR and merge it to the v1.x branch and not master |
|
In addition, let's open an issue for this and associate the PR with it so we don't leave this out in the next release notes |
vladshub
force-pushed
the
feat-invocation-logger
branch
from
d771b22
to
51ff719
Compare
vladshub
force-pushed
the
feat-invocation-logger
branch
from
51ff719
to
ddc9ba2
Compare
vladshub
force-pushed
the
feat-invocation-logger
branch
from
a56e8f0
to
8963173
Compare
Closes issue #87