[#375] Add log metrics #437

ale64bit · 2023-06-09T08:49:21Z

ale64bit commented

2023-06-09 08:49:21 +00:00

Signed-off-by: Alejandro Lopez a.lopez@yadro.com
Closes #375

Signed-off-by: Alejandro Lopez <a.lopez@yadro.com> Closes #375

ale64bit requested review from storage-core-committers 2023-06-09 08:49:32 +00:00

ale64bit requested review from storage-core-developers 2023-06-09 08:49:33 +00:00

dstepanov-yadro reviewed 2023-06-09 09:43:38 +00:00

pkg/util/logger/metrics.go Outdated

					
				@ -0,0 +29,4 @@

					}

				}

				func (m *logMetrics) Add(level zapcore.Level, dropped bool) {

dstepanov-yadro commented

2023-06-09 09:43:38 +00:00

I think Inc would be better because Add requires value to add i think.

I think `Inc` would be better because `Add` requires value to add i think.

ale64bit commented

2023-06-09 10:00:00 +00:00

done, although I didn't mean it in the "Add a delta" sense, but in the "Add a log entry" sense.

done, although I didn't mean it in the *"Add a delta"* sense, but in the *"Add a log entry"* sense.

dstepanov-yadro reviewed 2023-06-09 09:44:43 +00:00

pkg/util/logger/logger.go Outdated

					
				@ -83,3 +88,4 @@

					c.Level = lvl

					c.Encoding = "console"

					c.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder

					c.Sampling.Hook = func(e zapcore.Entry, sd zapcore.SamplingDecision) {

dstepanov-yadro commented

2023-06-09 09:44:43 +00:00

Will this Hook passed to nested loggers?

Will this `Hook` passed to nested loggers?

ale64bit commented

2023-06-09 10:00:41 +00:00

I don't know. Which nested loggers?

dstepanov-yadro commented

2023-06-09 10:45:52 +00:00

For example here: https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/local_object_storage/shard/id.go#L52

ale64bit commented

2023-06-09 10:56:50 +00:00

I see. Yeah, I just tried and it does propagate.
IMHO it would really strange if it didn't, from a user perspective. The hook is just a regular config option: why would it be skipped compared to other config options when calling With or cloning the logger?

I see. Yeah, I just tried and it does propagate. IMHO it would really strange if it didn't, from a user perspective. The hook is just a regular config option: why would it be skipped compared to other config options when calling `With` or cloning the logger?

dstepanov-yadro commented

2023-06-09 11:09:55 +00:00

I don't know, i've just asked)

👍 1

dstepanov-yadro marked this conversation as resolved

ale64bit force-pushed feature/375-log-metrics from 527cb9e480 to 55f9181c68

2023-06-09 09:59:59 +00:00

Compare

dstepanov-yadro approved these changes 2023-06-09 11:10:10 +00:00

acid-ant approved these changes 2023-06-13 05:27:59 +00:00

fyrchik approved these changes 2023-06-13 09:48:44 +00:00

fyrchik merged commit 0c40d98f7a into master