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

operator: remove invalid command flags like -loggerJSONFields #963

Closed
mbrancato opened this issue May 23, 2024 · 4 comments
Closed

operator: remove invalid command flags like -loggerJSONFields #963

mbrancato opened this issue May 23, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@mbrancato
Copy link

Is it possible for support for loggerJSONFields, as with other VM components, to be added to the operator? The operator seems to output in json by default, but the fields are not configurable from what I can tell.

I added this to the values.yaml for the Helm chart with no success using 0.44.0:

extraArgs:
  loggerFormat: json
  loggerJSONFields: "ts:timestamp,msg:message,level:severity"
@Haleygo
Copy link
Contributor

Haleygo commented May 23, 2024

Hi,
the extraArgs should work for each VM component. Can you see the -loggerJSONFields flag in pod's cmd after adding it to values.yaml?

@Haleygo Haleygo added the question Further information is requested label May 23, 2024
@Haleygo Haleygo self-assigned this May 23, 2024
@mbrancato
Copy link
Author

@Haleygo
I'm referring to the actual operator binary. It seems to output in JSON, but it doesn't support remapping fields. It lists it in the help, but I don't see the code to implement it and it definitely doesn't seem to work.

In other words, this image: https://hub.docker.com/r/victoriametrics/operator
or this release: https://github.com/VictoriaMetrics/operator/releases/tag/v0.44.0

I'm deploying with the helm chart and using extraArgs to pass the loggerJSONFields, resulting in a deployment with this:

  template:
    metadata:
      creationTimestamp: null
      labels:
        app.kubernetes.io/instance: victoria-metrics-operator
        app.kubernetes.io/name: victoria-metrics-operator
    spec:
      containers:
      - args:
        - --zap-log-level=info
        - --enable-leader-election
        - --loggerFormat=json
        - --loggerJSONFields=ts:timestamp,msg:message,level:severity
        command:
        - manager
        env:
        - name: WATCH_NAMESPACE
        - name: POD_NAME
          valueFrom:
            fieldRef:
              apiVersion: v1
              fieldPath: metadata.name
        - name: OPERATOR_NAME
          value: victoria-metrics-operator
        - name: VM_PSPAUTOCREATEENABLED
          value: "false"
        - name: VM_ENABLEDPROMETHEUSCONVERTEROWNERREFERENCES
          value: "false"
        image: victoriametrics/operator:v0.44.0

But the log fields are not remapped in the operator pod.

@Haleygo
Copy link
Contributor

Haleygo commented May 24, 2024

Okay, thanks for clarification.
Those flags are brought by github.com/VictoriaMetrics/VictoriaMetrics/lib/logger which we don't use directly inside operator, but used by other package like github.com/VictoriaMetrics/VictoriaMetrics/lib/promrelabel. Since we didn't Init the logger, those flags won't work.
And operator has it's own internal lib package github.com/VictoriaMetrics/operator/controllers/factory/logger, we need to do some cleanup about the flags.

@Haleygo Haleygo removed the question Further information is requested label May 24, 2024
@Haleygo Haleygo changed the title Add support for loggerJSONFields operator: remove invalid command flags like -loggerJSONFields May 24, 2024
@Haleygo Haleygo added the bug Something isn't working label May 24, 2024
@Haleygo Haleygo assigned f41gh7 and unassigned Haleygo May 29, 2024
f41gh7 added a commit that referenced this issue Jun 10, 2024
explicitly bind only needed flags to the manager.
removes VictoriaMetrics httpserver for debug pprof. Use built-in pprof manager httpserver
#963
Amper pushed a commit that referenced this issue Jun 10, 2024
* internal/manager: removes transitive dependency flags
explicitly bind only needed flags to the manager.
removes VictoriaMetrics httpserver for debug pprof. Use built-in pprof manager httpserver
#963

* fixes linter warnings

* Makefile: adds /internal packages to linter args
@f41gh7
Copy link
Collaborator

f41gh7 commented Jun 11, 2024

Changes was included to v0.45.0 release.

Operator now longer exposes VM related flags.

@f41gh7 f41gh7 closed this as completed Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants