feat(load-generator): Adapt OpenFeature OFREP instead of IN_PROCESS

[aepfli]

Changes

OpenFeature supports multiple different modes.
The newest implementation relies on threads and other python internal, which seem to be monkeypatched by locust.
Due to this, the IN_PROCESS mode is not working properly anymore and is causing issues.

Instead we suggest to switch to the OFREP (OpenFeature Remote Evaluation Protocol) for this usecase. This is a simple http call which will return all evaluate flags, and will regularly poll for changes.

relates:

• openfeature-provider-flagd unexpected behaviour open-feature/python-sdk-contrib#198
• Load Generator (locust) doesn't make the application emit traces #2051

Merge Requirements

For new features contributions, please make sure you have completed the following
essential items:

• CHANGELOG.md updated to document new feature additions
• Appropriate documentation updates in the docs
• Appropriate Helm chart updates in the helm-charts

Maintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: julianocosta89 / name: Juliano Costa (0bd7abf, 74eaa97, 0aaf37c, c0e272c)
• ✅ login: aepfli / name: Simon Schrottner (a70d7f3, 76c2080)

[aepfli]

i created open-telemetry/opentelemetry-helm-charts#1563 - but my helm knowledge is limited, if there is anything i can or should improve please let me know

[aepfli]

I checked the documentation, but i could not find any deeper explanation of the ports, nor the used modes for OpenFeature, hence i do not see any need for a documentation update. wdyt?

[julianocosta89]

@aepfli this one should be good to go.
In regards the Helm chart, it requires a couple of other steps, but I'll get back to you on the other PR.

I've added to your PR the flagd ports on the docker-compose.minimal.yml file, hope you don't mind.

Is there anything here you want us to wait before merging it?

[julianocosta89]

I've noticed that in the Helm update, you are adding FLAGD_OFREP_PORT to the load-generator, but not in here.
Is that missing?

[aepfli]

I've noticed that in the Helm update, you are adding FLAGD_OFREP_PORT to the load-generator, but not in here. Is that missing?

thank you nice catch, added the envvar to the load generator

[aepfli]

Is there anything here you want us to wait before merging it?

from my side we can go ahead with this pr ;) thank you

[julianocosta89]

@aepfli last question before getting this merged.

I was testing it out and noticed that the traces for the evaluation are not connected with load-generator:

Is there a way to connect the load-generator with the flagd call?

[aepfli]

okay here comes my limited OpenTelemetry know-how ;)

Yes there is, our OpenTelemetryHook implementation is really simple and we are "attaching" ourselves to the current span. the question is, who is responsible to create the current span, and should this maybe be done by the load-generator. see https://github.com/open-feature/python-sdk-contrib/blob/345e7934b9de3879d3aff45c8213ece1a98e3711/hooks/openfeature-hooks-opentelemetry/src/openfeature/contrib/hook/opentelemetry/__init__.py#L16-L42

As I said, my knowledge here is limited but am I on the right track, and should we generate a new span for each WebsiteUser invocation?

[julianocosta89]

Recommendation is doing this:

https://github.com/open-telemetry/opentelemetry-demo/blob/main/src/recommendation/recommendation_server.py#L132

[julianocosta89]

But Loadgen is also doing this:

https://github.com/open-telemetry/opentelemetry-demo/blob/main/src/load-generator/locustfile.py#L73

🤔

[aepfli]

we are never creating spans, only adding ourselves to existing ones. So I am not sure, if we should extract this into an own topic, and think about what is a good span for a loadtest. When to create a new one, etc. wdyt?

[julianocosta89]

Had some time again to look at this.
It seems that the Loadgen is creating the span:

But it looks like the context is not being propagated with the hook.

[aepfli]

most likely the span is generated with self.client.get("/") (https://github.com/open-telemetry/opentelemetry-demo/blob/main/src/load-generator/locustfile.py#L172) - but that is after flag evaluation, hence we do not have a span created when the flag is evaluated - or am i wrong here? question do you see the span correctly attached in the previous version?

There are two different things actually ;)
one is the request to fetch out flag configuration that is happening every 5s via polling, but that is detached from any kind of load generator behavior.
second is the actual evaluation of the flag, this is not related to the first one, and this is happening before the self.client.get.

I do not think it makes sense to combine both of them into one span. As the first is responsible for having flag data within the memory. Where as the second one is actually interesting for the span - when we are generating the load.

[julianocosta89]

Yeah, it makes sense to keep them as separate spans.

I wonder if the call shouldn't have a span and be linked to the flagd though

[beeme1mr]

I wonder if the call shouldn't have a span and be linked to the flagd though

What's the goal? The current behavior seems correct unless we want to manually create a span within the load generator.

[julianocosta89]

@beeme1mr the loadgen span is already being created no?!
#2114 (comment)

[aepfli]

@beeme1mr the loadgen span is already being created no?! #2114 (comment)

I was just assuming that client.get creates a log span, but if it is, this span is generated in that method, before we evaluate flags. hence i think this is an additional possible improvement, and should be most likely extracted into an own topic.

[julianocosta89]

@beeme1mr the loadgen span is already being created no?! #2114 (comment)

I was just assuming that client.get creates a log span, but if it is, this span is generated in that method, before we evaluate flags. hence i think this is an additional possible improvement, and should be most likely extracted into an own topic.

Totally! That would indeed be a different issue, I'm just discussing here because we are at it.
But let me get this one merged

[aepfli]

Totally! That would indeed be a different issue, I'm just discussing here because we are at it. But let me get this one merged

I will do a short exploration, maybe i can create an easy to use solution ;)

[aepfli]

see: #2119 - where i add spans around each task of the locust load generator