Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

utils/logger: support append user specified ID in log #4040

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

eryugey
Copy link
Contributor

@eryugey eryugey commented Sep 5, 2023

So that we could filter logs from the same juicefs instance. And use "--log-id random" to generate random uuid as log id.

So that we could filter logs from the same juicefs instance. And use
"--log-id random" to generate random uuid as log id.

Signed-off-by: Eryu Guan <[email protected]>
@SandyXSD
Copy link
Contributor

SandyXSD commented Sep 6, 2023

Usually we use pid to separate different clients. When do you think is that not enough?

@eryugey
Copy link
Contributor Author

eryugey commented Sep 6, 2023

When running in container, pid is always 1, so there's no way to distinguish different juicefs instances from pid. And logs from multiple juicefs containers might be collected to a central place, e.g. aliyun SLS, so log contents are merged together, we need log id to tell which log comes from which instance.

@davies
Copy link
Contributor

davies commented Sep 6, 2023

Can we added something to identify different containers when collecting the logging?

@eryugey
Copy link
Contributor Author

eryugey commented Sep 7, 2023

That should work too, but I'm not aware any standard way to do this (I think that depends on log services, and probably not all log services provide the required function). So adding log id in juicefs works everywhere.

@davies
Copy link
Contributor

davies commented Sep 8, 2023

The pid (1) is useless in container, so we can change it to use session id in the logging.

@eryugey
Copy link
Contributor Author

eryugey commented Sep 11, 2023

In my case Session ID is not that useful either, I have multiple containers in a Pod, e.g. 2 sidecar containers and 1 main container, I want to use a unique log id for the 3 containers, so I could know the logs with this log id came from the same pod.

Session ID could only identify juicefs, but I still didn't know which pod it was in. So a user specified log id is better. Maybe session ID could be the default log id, not a random uuid.

@davies
Copy link
Contributor

davies commented Sep 11, 2023

How do you use --log-id in your case?

@eryugey
Copy link
Contributor Author

eryugey commented Sep 11, 2023

How do you use --log-id in your case?

I use pod name as log id, e.g.

env:
      - name: POD_NAME
        valueFrom:
          fieldRef:
            fieldPath: metadata.name
      - name: POD_NS
        valueFrom:
          fieldRef:
            fieldPath: metadata.namespace
...
   args: juicefs mount ... --log-id "${POD_NS}-${POD_NAME}"

@davies
Copy link
Contributor

davies commented Sep 20, 2023

Sounds reasonable, merging it, thanks!

@davies davies merged commit 21c44e3 into juicedata:main Sep 20, 2023
@eryugey eryugey deleted the logid branch September 20, 2023 05:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants