Skip to content

Conversation

dottorblaster
Copy link

Still have to fill everything properly and this is just very much a draft, beware!

Changes

Please provide a brief description of the changes here.

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.

@dottorblaster dottorblaster requested a review from a team as a code owner August 5, 2025 21:25
Copy link

linux-foundation-easycla bot commented Aug 5, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@dottorblaster dottorblaster marked this pull request as draft August 5, 2025 21:29
@dottorblaster dottorblaster changed the title [DRAFT] Flagd UI elixir rewrite [DRAFT] Flagd UI Elixir rewrite Aug 5, 2025
# Changes to config/runtime.exs don't require recompiling the code
COPY config/runtime.exs config/

COPY rel rel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that this is still in DRAFT, but I wanted to take a look and the docker build failed here.

"/rel": not found

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking a look as soon as I can, thanks 😄

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@julianocosta89 building was broken when using the docker compose setup, sorry 😓 I pushed a commit to fix it 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still got the same error 😢

target flagd-ui: failed to solve: failed to compute cache key: failed to calculate checksum of ref f62ab0d2-6f18-44c0-a488-f189e8e4e313::9w7lyomo2ina5sp6dlak0k74p: "/src/flagd-ui/rel": not found

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@julianocosta89 I don't know if you tried with the updated branch but if you didn't, feel free to try again and let me know!

I know "it works on my machine" isn't much, but indeed 😂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe LiveView does not get instrumented through the phoenix instrumentation library.

The issue is that LiveView is a long lived websocket connection doing bidirectional communication, so not a clean set of requests/response spans, and no one has spent the time to define out what those spans and events should be by default, instead leaving it up to the user to create spans where they see fit. There is likely another similar lib in another language we can piggy back on or work with.

That said... this looks like just a GET /feature request? Does it then setup a LiveView channel so that is why it isn't instrumented by the default Phoenix Otel library?

Copy link
Author

@dottorblaster dottorblaster Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsloughter exactly, the connection gets upgraded to a websocket/LiveView one. Do you have any hint about how can I produce better (linked to this context) traces myself or any other tool I can pick up from the ecosystem?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsloughter @julianocosta89 I think I figured out what was going on, in addition to some tiny fixes I also realized I was operating under the assumption the app was using Cowboy while indeed we have to leverage Bandit instrumentation. Mega facepalm on my end...

We should be able to see linked traces now! Can you try again?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dottorblaster when trying to run it, I wasn't able to load the /feature.

Got this response from envoy:

no healthy upstream

When checking flagd-ui logs I got this:

=WARNING REPORT==== 15-Sep-2025::08:36:59.674257 ===
OTLP exporter failed to initialize with exception error:{badmatch,
                                                         {error,
                                                          inets_not_started}}
=WARNING REPORT==== 15-Sep-2025::08:37:11.024393 ===
OTLP exporter failed to initialize with exception error:{badmatch,
                                                         {error,
                                                          inets_not_started}}
=WARNING REPORT==== 15-Sep-2025::08:37:32.246167 ===
OTLP exporter failed to initialize with exception error:{badmatch,
                                                         {error,
                                                          inets_not_started}}
08:37:32.266 [info] Loading 129 CA(s) from :otp store
08:37:32.277 [info] Read new state from file

I had to restart the flogd-ui to make it work.
After restarting I got the following:

=WARNING REPORT==== 15-Sep-2025::08:36:59.674257 ===
OTLP exporter failed to initialize with exception error:{badmatch,
                                                         {error,
                                                          inets_not_started}}
=WARNING REPORT==== 15-Sep-2025::08:37:11.024393 ===
OTLP exporter failed to initialize with exception error:{badmatch,
                                                         {error,
                                                          inets_not_started}}
=WARNING REPORT==== 15-Sep-2025::08:37:32.246167 ===
OTLP exporter failed to initialize with exception error:{badmatch,
                                                         {error,
                                                          inets_not_started}}
08:37:32.266 [info] Loading 129 CA(s) from :otp store
08:37:32.277 [info] Read new state from file
=WARNING REPORT==== 15-Sep-2025::08:41:53.912102 ===
OTLP exporter failed to initialize with exception error:{badmatch,
                                                         {error,
                                                          inets_not_started}}
08:41:53.940 [info] Loading 129 CA(s) from :otp store
08:41:53.950 [info] Read new state from file
08:41:53.959 [info] Running FlagdUiWeb.Endpoint with Bandit 1.7.0 at :::4000 (http)
08:41:53.959 [info] Access FlagdUiWeb.Endpoint at https://localhost
08:41:54.363 [notice] SIGTERM received - shutting down

Not sure if that was caused by the fact that it took some time to the Collector to be healthy.
Either way it would be good to have some way of ensuring the flagd-ui is always started.

After restarting I was able to see the trace:
image

One other intriguing thing is that the flagd-ui span is GET / and not GET /feature 🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if that was caused by the fact that it took some time to the Collector to be healthy.

Yes definitely @julianocosta89, it's because of that. Maybe we can just wait until the collector starts, it looks like an easy fix 😃

One other intriguing thing is that the flagd-ui span is GET / and not GET /feature 🤔

That's because the service is designed/configured to be deployed behind a reverse proxy and the route just gets rewritten. Unless we rewrite all the routing it can't be otherwise 😅

@github-actions github-actions bot added the helm-update-required Requires an update to the Helm chart when released label Aug 8, 2025
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added Stale and removed Stale labels Aug 16, 2025
@dottorblaster dottorblaster force-pushed the flagd-ui-elixir-rewrite branch 2 times, most recently from 04758cc to 473c829 Compare September 1, 2025 08:21
@dottorblaster dottorblaster marked this pull request as ready for review September 1, 2025 08:23
@dottorblaster dottorblaster changed the title [DRAFT] Flagd UI Elixir rewrite Flagd UI Elixir rewrite Sep 1, 2025
@dottorblaster dottorblaster force-pushed the flagd-ui-elixir-rewrite branch from afb1052 to 2768a9d Compare September 6, 2025 08:02
@dottorblaster dottorblaster force-pushed the flagd-ui-elixir-rewrite branch from 2768a9d to 22673c9 Compare September 6, 2025 08:02
@dottorblaster dottorblaster force-pushed the flagd-ui-elixir-rewrite branch from 22673c9 to 681d46b Compare September 6, 2025 08:06
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
helm-update-required Requires an update to the Helm chart when released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants