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

Enable adding user-defined environment variables to collector/query #396

Closed
kacper-jackiewicz opened this issue May 6, 2019 · 11 comments
Closed

Comments

@kacper-jackiewicz
Copy link

During the testing I found out that there is no possibility to update the environment variable part of deployment.
From what I am seeing in the operator code that part of deployment spec is constant, in the meaning of hard coded list of variables which take changeable values from jaeger cr spec.

My questions are:

  • Am I missing something? Is there any possible way to add additional env variable for example go specific variables like GO_MAX_PROC?
  • Is this done deliberately or will it be accounted for in the near future? If it is on purpose what is the exact reason for disabling env variables changes.
@pavolloffay
Copy link
Member

It's a design decision. The operator does not allow users to change created objects. All changes are reverted.

All configuration is only available via jaeger CR.

@kacper-jackiewicz
Copy link
Author

Is there a possibility to add env variables through Jeager CR?

@pavolloffay
Copy link
Member

I don't think there is. Let's take this by use cases what properties would you like make configurable?

@kacper-jackiewicz
Copy link
Author

kacper-jackiewicz commented May 6, 2019

First thing would be to have ability to add new environment variables per component e.g.

apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
  name: jaeger
spec:
  strategy: production
  storage:
  ...
  collector:
    replicas: 2
    options:
    ...
    env:
    - name: GOMAXPROCS:
       value: '5'
   
   and additional variables needed

@pavolloffay
Copy link
Member

That would allow users to arbitrary change jaeger configuration. Is the goal to configure only GOMAXPROCS? The operator should set this variable automatically if it is needed.

@kacper-jackiewicz
Copy link
Author

From my side for now it is only GOMAXPROCS. In case that this controlled by operator I will not need to change that. Thanks for quick response.

@pavolloffay
Copy link
Member

GOMAXPROCS sets the maximum number of CPUs that can be executing simultaneously and returns the previous setting. If n < 1, it does not change the current setting. The number of logical CPUs on the local machine can be queried with NumCPU. This call will go away when the scheduler improves.

Do you have any recommendations on the value? Should we set it to resource requests?

@kacper-jackiewicz
Copy link
Author

kacper-jackiewicz commented May 6, 2019

In our deployments, GOMAXPROCS value was always matching the resource limits, but in that case request limits/requests need to be fixed

@pavolloffay
Copy link
Member

I am wondering whether this library https://github.com/uber-go/automaxprocs would offer a better solution rather than setting it explicitly on the manifest.

@kacper-jackiewicz
Copy link
Author

It seems like it would be more flexible solution.

@jpkrohling
Copy link
Contributor

I'm closing this, as it was added to Jaeger itself. It should be available whenever the next version is released, probably 1.13.

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

No branches or pull requests

3 participants