storj / edge

Storj edge services (including multi-tenant, S3-compatible server to interact with the Storj network)
GNU Affero General Public License v3.0
48 stars 18 forks source link

logging clocks order issue #352

Closed amwolff closed 11 months ago

amwolff commented 11 months ago

Expected Behavior

The expected behavior is that when running the provided code, the clocks should be logged in a sorted order based on their keys (ids). For example:

id: app11, val: 123
id: app12, val: 456
id: app21, val: 789
id: app22, val: 999999
id: app31, val: 0

Code

package main

import (
    "log"
    "sort"
)

type clocksLogObject map[string]uint64

func (o clocksLogObject) MarshalLogObject() error {
    var (
        ids  []string
        vals []uint64
    )
    for id, val := range o {
        ids, vals = append(ids, id), append(vals, val)
    }
    f := func(i, j int) bool { return ids[i] < ids[j] }
    sort.Slice(vals, f)
    sort.Slice(ids, f)
    for i, id := range ids {
        log.Printf("id: %s, val: %d", id, vals[i])
    }
    return nil
}

func main() {
    clocksLogObject := make(clocksLogObject)
    clocksLogObject["app11"] = 123
    clocksLogObject["app12"] = 456
    clocksLogObject["app21"] = 789
    clocksLogObject["app22"] = 999999
    clocksLogObject["app31"] = 0
    clocksLogObject.MarshalLogObject()
}

Current Behavior

Currently, when running the provided code, the clocks are logged in a seemingly random order due to the dependent sorting of slices.

Possible Solution

...

Steps to Reproduce

  1. Copy the provided code into a Go code file.
  2. Run the Go program.

Context (Environment)

This issue affects the developers of Auth Service. It may cause confusion and difficulty in analyzing the logs since the order of the logs is not consistent.

Detailed Description

The current implementation of the MarshalLogObject() function relies on sorting two slices, ids and vals, based on the ids. However, sorting both slices using the same sorting function creates a dependency between them, leading to unpredictable and incorrect results.

A better approach would be to sort only the ids slice independently while keeping the corresponding values in sync. This can be achieved by creating a custom sorting method that sorts the ids slice and then uses the sorted order of ids to access the corresponding values in the vals slice. By doing this, the clocks will be logged in the expected sorted order.

Possible Implementation

...

storj-gerrit[bot] commented 11 months ago

Change pkg/auth/badgerauth: fix incorrect logging of clocks mentions this issue.