Logger: Support write to json file as the same time in fluent logger #1470
Conversation
Zheaoli
force-pushed
the
manjusaka/support-1374
branch
from
7889165 to
2d7c0ec
Compare
cmd/nerdctl/run_linux_test.go
Outdated
| @@ -247,7 +247,7 @@ func TestRunWithFluentdLogDriverWithLogOpt(t *testing.T) { | |||
| time.Sleep(3 * time.Second) | |||
|
|
|||
| testContainerName := containerName + "test" | |||
| base.Cmd("run", "-d", "--log-driver", "fluentd", "--log-opt", "fluentd-address=127.0.0.1:24225", | |||
| base.Cmd("run", "-d", "--log-driver", "fluentd", "--log-opt", "fluentd-address=127.0.0.1:24225", "--log-opt", "fluentd-log-to-json=true", | |||
There was a problem hiding this comment.
Thanks for the change. I am not sure why this feature is required:
- It is already supported by fluentd as form of multiple outputs (https://docs.fluentd.org/output/secondary_file, https://docs.fluentd.org/output/file). Is there a use-case?
- It is not supported in docker.
- Even stdout is supported by fluentd (https://docs.fluentd.org/output/stdout) whichis what this test is doing.
if you want to still merge the change and maintainers are fine, can you update the readme.
There was a problem hiding this comment.
Sorry for replying late
- Help people to use
nerdctl logsto get the container log(quickly debug, not need to go to the fluent backend server to get the log) - Yep, this is a nerdctl-only feature
- Yep, but in common use case, the people cannot decide whether the fluentd use stdout or not
There was a problem hiding this comment.
Could this PR be more generic then (i.e for all loggers except default and json)? There would be other remote log forwarders that can make use of this functionality.
There was a problem hiding this comment.
emmmm, I want to split it into different PR,#1374
There was a problem hiding this comment.
Hmm! Would you copy the same code in every logger then? Is it possible to make the code generic for any logger and have it in a single place so that when new loggers are added; they automatically inherit this logic rather than have more changes in the code base.
pkg/logging/fluentd_logger.go
Outdated
| @@ -110,21 +117,52 @@ func (f *FluentdLogger) Process(_ string, config *logging.Config) error { | |||
| if err != nil { | |||
| return fmt.Errorf("failed to create fluent client: %w", err) | |||
| } | |||
| var jsonLogger *logrotate.Logger | |||
There was a problem hiding this comment.
Out of curiosity: Is it not possible to exec the nerdctl log driver or call the go routine for json driver run directl.
Zheaoli
force-pushed
the
manjusaka/support-1374
branch
from
1095ede to
8abfbfa
Compare
Zheaoli
force-pushed
the
manjusaka/support-1374
branch
from
457481e to
5efcf53
Compare
fix format error Signed-off-by: Manjusaka <[email protected]>
Zheaoli
force-pushed
the
manjusaka/support-1374
branch
from
5efcf53 to
3071a69
Compare
|
@yankay are you still interested in carrying this through? |
Let me investigate it :-) |
|
The feature is not supported by docker https://docs.docker.com/engine/logging/drivers/fluentd/. |
No description provided.