[installer] There is no option to set a sampling strategy in the observability section

[aledbf]

Bug description

Right now we trace 100% of the events. This could be an issue in SaaS options (like honeycomb)

Steps to reproduce

Initialize and render the manifests, using the installer.
Check JAEGER_SAMPLER_TYPE is configured to const and JAEGER_SAMPLER_PARAM is 1

Workspace affected

No response

Expected behavior

No response

Example repository

No response

Anything else?

No response

[aledbf]

/team self-hosted

[roboquat]

@aledbf: The label(s) team: self-hosted cannot be applied. These labels are supported: team: IDE, team: meta, team: workspace

In response to this:

/team self-hosted

Instructions for interacting with me are available here. If you have questions or suggestions related to my behavior, please file an issue against the gitbot repository.

[mrsimonemms]

Have spoken to @aledbf about this requirement and suggest this as the new items in the config.yaml

observability:
  tracing:
    samplerType: ratelimiting
    samplerParam: 10.0

These will then be converted into the values for envvars in the TracingEnv function

Questions:

1. The samplerType will be evaluated as a string. Do we need to validate a set of enumerable values as well?
2. The sampleParam will be evaluated as a number (and then converted to string by the TracingEnv function). Do these need to have any boundaries set, perhaps >= 0 && <=100?

[aledbf]

Good question. If we use the Jaeger env variables, then, maybe we can use the patterns from the docs? https://www.jaegertracing.io/docs/1.29/sampling/#client-sampling-configuration
or add a kind for Jaeger and deserialize to the config type?

observability:
  tracing:
    kind: Jaeger
    config:
      sampler:
        type: ratelimiting
        param: 10.0

[mrsimonemms]

I'll have a look at that and see what I can do. I guess the other alternative is to have no validation in for the moment and do that as a ticket for future work.

Will read the docs and put my thoughts in a PR

[corneliusludmann]

Thanks, @aledbf, for raising this issue. Simon and I just discussed this and I want to understand better what we need for SaaS here.

For self-hosted, we won't support setting up Jaeger currently. That's why we would prefer a solution that doesn't need an installer config change (to keep the config surface concise). However, the question here is, do we (for SaaS) need to change this value in the config file, or would it be sufficient to change the value in the code to a sensible value.

Thus, the questions are:

1. What value do we need for SaaS? Is it always 10.0 ratelimiting?
2. Will this value change rather often or would it be sufficient when we change the values as constants in the code to the value we need (and change the constant in the code once we need another value)?

[aledbf]

we need for SaaS here.
For self-hosted, we won't support setting up Jaeger currently.

I think this is beyond the flavor/context of the installation. As a product, Gitpod has components that expose observability data. We need to configure that, somehow. If that's not the case, it must be removed (I know, that's not an option).
Providing the settings to configure the observability features != providing support for the chosen implementation.

However, the question here is, do we (for SaaS) need to change this value in the config file, or would it be sufficient to change the value in the code to a reasonable value.

The configuration spreads across several deployments/daemonsets. Adjusting values in manifests is too painful/hard to maintain.

What value do we need for SaaS? Is it always 10.0 ratelimiting?

Let's forget for a second about Gitpod SaaS. The value depends on how you run your monitoring infrastructure. If it's in-house, you can maybe trace 100% of the traffic, but if you use a third-party vendor, it depends on how big your wallet is.
For that reason, this must be adjustable.

[corneliusludmann]

The configuration spreads across several deployments/daemonsets. Adjusting values in manifests is too painful/hard to maintain.

I don't propose adjusting values in the manifest files at all. That should be an ultimate exception. I propose to don't make this configurable but set a proper value hard-coded in the installer.

This brings us to the actual question that we need to answer first …

I think this is beyond the flavor/context of the installation. As a product, Gitpod has components that expose observability data. We need to configure that, somehow. If that's not the case, it must be removed (I know, that's not an option).
Providing the settings to configure the observability features != providing support for the chosen implementation.

I'm not talking about the chosen implementation (I guess you mean Jaeger here) but about overall observability (which is traceability here, right?). If I understand @metcalfc correctly, we don't want to officially support this for self-hosted (correct me if I'm wrong, Chad). That means, we expose observability data (because we use it) but we have no ambitions to support our self-hosted customers to use it.

Or is there anything (technically) that speaks against it?

Given that it's only SaaS that is important here and we don't need to make it configurable but just set the values we need for SaaS.

[mrsimonemms]

I think it's subtly different to what we're supporting. I don't think we want to support the creation/management of the monitoring stack. This change would support what we're publishing to a generic stack which I think is fine. I could see this as being important for self-hosted installations as well, especially the larger deployments.

I think of this as a pub/sub model - we're allowing configuration of the publisher without making any comment on the subscriber

[mrsimonemms]

@mrsimonemms consider adding to experimental object in config