[#606] Support log tagging #606
Labels
No labels
P0
P1
P2
P3
good first issue
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-s3-gw#606
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "nzinkevich/frostfs-s3-gw:feature/log_tagging"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Signed-off-by: Nikita Zinkevich n.zinkevich@yadro.com
[#XX] Support log taggingto [#606] Support log taggingcc4db2cf96
to94b1c6ffd0
94b1c6ffd0
tofc6e4423cd
@ -302,3 +302,3 @@
args.IfUnmodifiedSince = httpTime
} else {
log.Warn(logs.FailedToParseHTTPTime, zap.String(api.IfUnmodifiedSince, headers.Get(api.IfUnmodifiedSince)), zap.Error(err))
log.Warn(logs.FailedToParseHTTPTime, zap.String(api.IfUnmodifiedSince, headers.Get(api.IfUnmodifiedSince)), zap.Error(err), logs.TagField(logs.TagApp))
I would mark this as
datapath
@ -94,2 +94,3 @@
zap.String("cnrID", corsBkt.CID.EncodeToString()),
zap.String("objID", addr.Object().EncodeToString()))
zap.String("objID", addr.Object().EncodeToString()),
logs.TagField(logs.TagExternalStorageTree))
Tag should be
logs.TagExternalStorage
I suppose@ -852,3 +854,3 @@
}
n.reqLogger(ctx).Info(logs.ResolveBucket, zap.Stringer("cid", cnrID))
n.reqLogger(ctx).Info(logs.ResolveBucket, zap.Stringer("cid", cnrID), logs.TagField(logs.TagExternalStorageTree))
Should we use
external_blockain
tag here?@ -81,2 +81,3 @@
zap.String("cid", lifecycleBkt.CID.EncodeToString()),
zap.String("oid", addr.Object().EncodeToString()))
zap.String("oid", addr.Object().EncodeToString()),
logs.TagField(logs.TagExternalStorageTree),
Probably it would be better to use
external_storage
@ -213,3 +213,3 @@
encInfo := FormEncryptionInfo(multipartInfo.Meta)
if err := p.Info.Encryption.MatchObjectEncryption(encInfo); err != nil {
n.reqLogger(ctx).Warn(logs.MismatchedObjEncryptionInfo, zap.Error(err))
n.reqLogger(ctx).Warn(logs.MismatchedObjEncryptionInfo, zap.Error(err), logs.TagField(logs.TagApp))
Should it be
datapath
?@ -291,3 +297,3 @@
n.reqLogger(ctx).Debug(logs.UploadPart,
zap.String("multipart upload", p.Info.UploadID), zap.Int("part number", p.PartNumber),
zap.Stringer("cid", bktInfo.CID), zap.Stringer("oid", createdObj.ID))
zap.Stringer("cid", bktInfo.CID), zap.Stringer("oid", createdObj.ID), logs.TagField(logs.TagExternalStorage))
Probably this should be
datapath
@ -481,2 +488,3 @@
zap.String("uploadKey", p.Info.Key),
zap.Error(err))
zap.Error(err),
logs.TagField(logs.TagExternalStorage))
Probably this can be
datapath
@ -299,0 +298,4 @@
n.reqLogger(ctx).Debug(logs.FailedToDeleteObject,
zap.Stringer("cid", p.BktInfo.CID),
zap.Stringer("oid", createdObj.ID),
logs.TagField(logs.TagApp),
Why not
external_storage
?@ -313,0 +316,4 @@
n.reqLogger(ctx).Debug(logs.FailedToDeleteObject,
zap.Stringer("cid", p.BktInfo.CID),
zap.Stringer("oid", createdObj.ID),
logs.TagField(logs.TagApp))
Why not
external_storage
?@ -140,3 +140,3 @@
containerID, err := resolveContainerID(ctx, reqInfo.BucketName)
if err != nil {
reqLogOrDefault(ctx, log).Debug(logs.FailedToResolveCID, zap.Error(err))
reqLogOrDefault(ctx, log).Debug(logs.FailedToResolveCID, zap.Error(err), logs.TagField(logs.TagApp))
external_blockchain
?@ -202,2 +202,3 @@
zap.String("resource", res), zap.Any("request properties", requestProps),
zap.Any("resource properties", resourceProps))
zap.Any("resource properties", resourceProps),
logs.TagField(logs.TagApp),
Should we use
datapath
tag?@ -697,16 +747,17 @@ func (a *App) initTracing(ctx context.Context) {
Version: version.Version,
}
log := a.log.With(logs.TagField(logs.TagConfig))
I would use
app
tag@ -799,2 +851,2 @@
prm.SetLogger(a.log)
prmTree.SetLogger(a.log)
prm.SetLogger(log)
prmTree.SetLogger(log)
Actually, I'm not sure if we need to pass
app
tagged logger to pool. Probably we need separate tag for thatWhy not using
external_storage_*
?@ -435,1 +435,3 @@
l.Fatal(logs.FailedToParseDefaultDefaultLocationConstraint, zap.String("policy", defaultPlacementPolicy))
l.Fatal(logs.FailedToParseDefaultDefaultLocationConstraint,
zap.String("policy", defaultPlacementPolicy),
logs.TagField(logs.TagConfig),
Probably any
Fatal
must beapp
tagged@alexvanin what do you think?
@ -509,3 +518,3 @@
regionPolicyMap, err := readRegionMap(filepath)
if err != nil {
l.Warn(logs.FailedToReadRegionMapFilePolicies, zap.String("file", filepath), zap.Error(err))
l.Warn(logs.FailedToReadRegionMapFilePolicies, zap.String("file", filepath), zap.Error(err), logs.TagField(logs.TagApp))
This should have
config
tag. In other places too.Add
app
tag to logs that initialize something app related (not warn/info about config value/parsing etc).@ -636,3 +649,3 @@
if len(defaultNamespaces) == 0 {
defaultNamespaces = defaultDefaultNamespaces
l.Warn(logs.DefaultNamespacesCannotBeEmpty, zap.Strings("namespaces", defaultNamespaces))
l.Warn(logs.DefaultNamespacesCannotBeEmpty, zap.Strings("namespaces", defaultNamespaces), logs.TagField(logs.TagApp))
This should have
config
tag.@ -660,3 +673,3 @@
nsConfig, err := readNamespacesConfig(v.GetString(cfgNamespacesConfig))
if err != nil {
l.Warn(logs.FailedToParseNamespacesConfig, zap.Error(err))
l.Warn(logs.FailedToParseNamespacesConfig, zap.Error(err), logs.TagField(logs.TagApp))
This should have
config
tag.@ -674,3 +687,3 @@
if len(overrideDefaults) > 0 {
l.Warn(logs.DefaultNamespaceConfigValuesBeOverwritten)
l.Warn(logs.DefaultNamespaceConfigValuesBeOverwritten, logs.TagField(logs.TagApp))
This should have
config
tag.@ -721,3 +734,3 @@
if address == "" {
l.Warn(logs.SkipEmptyAddress)
l.Warn(logs.SkipEmptyAddress, logs.TagField(logs.TagApp))
This should have
config
tag@ -737,2 +750,3 @@
zap.String("address", address),
zap.Float64("weight", weight))
zap.Float64("weight", weight),
logs.TagField(logs.TagApp),
This should have
config
tag@ -3,1 +16,4 @@
return zap.String(TagFieldName, tag)
}
const (
Please group log message by tag groups https://git.frostfs.info/TrueCloudLab/frostfs-s3-lifecycler/src/branch/master/internal/logs/logs.go
fc6e4423cd
tof4901b830f
f4901b830f
to14458cfdaf
14458cfdaf
to2f4f60b2ff
Missed logs
n.reqLogger(ctx).Warn(logs.MismatchedObjEncryptionInfo, zap.Error(err))
log.Warn(logs.FailedToParseHTTPTime, zap.String(api.IfUnmodifiedSince, headers.Get(api.IfUnmodifiedSince)), zap.Error(err))
@ -109,3 +109,3 @@
bktInfo, err := h.getBucketInfo(ctx, reqInfo.BucketName)
if err != nil {
h.reqLogger(ctx).Warn(logs.GetBucketInfo, zap.Error(err))
h.reqLogger(ctx).Warn(logs.GetBucketInfo, zap.Error(err), logs.TagField(logs.TagExternalBlockchain))
Probably
external_object_storage
be betterI still think we should use
external_object_storage
tag here@ -545,2 +545,3 @@
zap.Stringer("cid", p.BktInfo.CID),
zap.Stringer("oid", objInfo.ObjectInfo.ID))
zap.Stringer("oid", objInfo.ObjectInfo.ID),
logs.TagField(logs.TagExternalStorageTree),
I would use
datapath
@ -816,3 +868,3 @@
p, err := pool.NewPool(prm)
if err != nil {
a.log.Fatal(logs.FailedToCreateConnectionPool, zap.Error(err))
log.Fatal(logs.FailedToCreateConnectionPool, zap.Error(err), logs.TagField(logs.TagApp))
Please, don't use
log
package. Usea.log
@ -671,3 +695,3 @@
copiesNums[constraint] = vector32
l.Info(logs.ConstraintAdded, zap.String("location", constraint), zap.Strings("copies numbers", vector))
l.Info(logs.ConstraintAdded, zap.String("location", constraint), zap.Strings("copies numbers", vector), logs.TagField(logs.TagApp))
It's better to use
config
tag@ -60,6 +60,12 @@ logger:
initial: 100
thereafter: 100
interval: 1s
tags:
We should add new param to docs https://git.frostfs.info/nzinkevich/frostfs-s3-gw/src/branch/feature/log_tagging/docs/configuration.md
@ -540,3 +540,3 @@
ind := latest.GetLatestNodeIndex()
if latest.IsSplit() {
c.reqLogger(ctx).Error(logs.BucketSettingsNodeHasMultipleIDs, zap.Uint64s("ids", latest.ID))
c.reqLogger(ctx).Error(logs.BucketSettingsNodeHasMultipleIDs, zap.Uint64s("ids", latest.ID), logs.TagField(logs.TagApp))
I'm not sure if it should be
app
tag.Below we use
external_storage_tree
anddatapath
for the same case. Let's pick one2f4f60b2ff
to9056e93a54
9056e93a54
tof71e35850f
f71e35850f
toe1ee48827d
e1ee48827d
toe4d4ac075c
e4d4ac075c
to75cdff9036
75cdff9036
to86b638adf0
@ -63,0 +63,4 @@
tags:
- name: "app"
level: "debug"
- name: "config"
We don't have config tag now (the same for .env and configuration.md files)
86b638adf0
to4c20b224bb
4c20b224bb
to969d36ff6b
969d36ff6b
to7905aa3bf3
7905aa3bf3
toe7f620f137