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

[frontend] Slowloading of images #1515

Merged
merged 2 commits into from
May 13, 2024

Conversation

klucsik
Copy link
Contributor

@klucsik klucsik commented Apr 10, 2024

Changes

Add FeatureFlag Provider to client side.
Add http fault filter to envoy
Add slowloading featureflag, and modify the Product card element to disable caching and add the envoy http-fault header
Example of a slowly loaded image trace:
image

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.

@github-actions github-actions bot added docs-update-required Requires documentation update helm-update-required Requires an update to the Helm chart when released labels Apr 10, 2024
@klucsik klucsik marked this pull request as ready for review April 10, 2024 08:37
@klucsik klucsik requested a review from a team April 10, 2024 08:37
src/flagd/demo.flagd.json Outdated Show resolved Hide resolved
src/frontend/components/ProductCard/ProductCard.tsx Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
src/frontend/components/ProductList/ProductList.tsx Outdated Show resolved Hide resolved
Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

When trying to build and run this I'm getting the following error:

flagd                    | 2024-04-10T12:45:39.071Z	error	evaluator/fractional.go:30	parse fractional evaluation data: bucketing value not supplied and no targetingKey in context
flagd                    | github.com/open-feature/flagd/core/pkg/evaluator.(*Fractional).Evaluate
flagd                    | 	/src/core/pkg/evaluator/fractional.go:30
flagd                    | github.com/diegoholiveira/jsonlogic/v3.operation
flagd                    | 	/go/pkg/mod/github.com/diegoholiveira/jsonlogic/[email protected]/operation.go:15
flagd                    | github.com/diegoholiveira/jsonlogic/v3.apply
flagd                    | 	/go/pkg/mod/github.com/diegoholiveira/jsonlogic/[email protected]/jsonlogic.go:493
flagd                    | github.com/diegoholiveira/jsonlogic/v3.ApplyInterface
flagd                    | 	/go/pkg/mod/github.com/diegoholiveira/jsonlogic/[email protected]/jsonlogic.go:587
flagd                    | github.com/diegoholiveira/jsonlogic/v3.Apply
flagd                    | 	/go/pkg/mod/github.com/diegoholiveira/jsonlogic/[email protected]/jsonlogic.go:522
flagd                    | github.com/open-feature/flagd/core/pkg/evaluator.(*JSON).evaluateVariant
flagd                    | 	/src/core/pkg/evaluator/json.go:339
flagd                    | github.com/open-feature/flagd/core/pkg/evaluator.resolve[...]
flagd                    | 	/src/core/pkg/evaluator/json.go:276
flagd                    | github.com/open-feature/flagd/core/pkg/evaluator.(*JSON).ResolveAllValues
flagd                    | 	/src/core/pkg/evaluator/json.go:159
flagd                    | github.com/open-feature/flagd/core/pkg/service/flag-evaluation.(*FlagEvaluationService).ResolveAll
flagd                    | 	/src/core/pkg/service/flag-evaluation/flag_evaluator_v2.go:65
flagd                    | connectrpc.com/connect.NewUnaryHandler[...].func1
flagd                    | 	/go/pkg/mod/connectrpc.com/[email protected]/handler.go:52
flagd                    | connectrpc.com/connect.NewUnaryHandler[...].func2
flagd                    | 	/go/pkg/mod/connectrpc.com/[email protected]/handler.go:84
flagd                    | connectrpc.com/connect.(*Handler).ServeHTTP
flagd                    | 	/go/pkg/mod/connectrpc.com/[email protected]/handler.go:265
flagd                    | buf.build/gen/go/open-feature/flagd/connectrpc/go/flagd/evaluation/v1/evaluationv1connect.NewServiceHandler.func1
flagd                    | 	/go/pkg/mod/buf.build/gen/go/open-feature/flagd/connectrpc/[email protected]/flagd/evaluation/v1/evaluationv1connect/evaluation.connect.go:245
flagd                    | net/http.HandlerFunc.ServeHTTP
flagd                    | 	/usr/local/go/src/net/http/server.go:2166
flagd                    | github.com/open-feature/flagd/core/pkg/service/flag-evaluation.bufSwitchHandler.ServeHTTP
flagd                    | 	/src/core/pkg/service/flag-evaluation/connect_service.go:51
flagd                    | github.com/open-feature/flagd/core/pkg/service/flag-evaluation.(*ConnectService).setupServer.(*ConnectService).AddMiddleware.Middleware.Handler.func1.1
flagd                    | 	/src/core/pkg/service/middleware/metrics/http_metrics.go:105
flagd                    | github.com/open-feature/flagd/core/pkg/service/middleware/metrics.Middleware.Measure
flagd                    | 	/src/core/pkg/service/middleware/metrics/http_metrics.go:90
flagd                    | github.com/open-feature/flagd/core/pkg/service/flag-evaluation.(*ConnectService).setupServer.(*ConnectService).AddMiddleware.Middleware.Handler.func1
flagd                    | 	/src/core/pkg/service/middleware/metrics/http_metrics.go:104
flagd                    | net/http.HandlerFunc.ServeHTTP
flagd                    | 	/usr/local/go/src/net/http/server.go:2166
flagd                    | github.com/open-feature/flagd/core/pkg/service/flag-evaluation.(*ConnectService).setupServer.(*ConnectService).AddMiddleware.Middleware.Handler.(*Cors).Handler.func2
flagd                    | 	/go/pkg/mod/github.com/rs/[email protected]/cors.go:281
flagd                    | net/http.HandlerFunc.ServeHTTP
flagd                    | 	/usr/local/go/src/net/http/server.go:2166
flagd                    | golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP
flagd                    | 	/go/pkg/mod/golang.org/x/[email protected]/http2/h2c/h2c.go:125
flagd                    | net/http.serverHandler.ServeHTTP
flagd                    | 	/usr/local/go/src/net/http/server.go:3137
flagd                    | net/http.(*conn).serve
flagd                    | 	/usr/local/go/src/net/http/server.go:2039

@klucsik
Copy link
Contributor Author

klucsik commented Apr 10, 2024

When trying to build and run this I'm getting the following error:

flagd                    | 2024-04-10T12:45:39.071Z	error	evaluator/fractional.go:30	parse fractional evaluation data: bucketing value not supplied and no targetingKey in context
flagd                    | github.com/open-feature/flagd/core/pkg/evaluator.(*Fractional).Evaluate
flagd                    | 	/src/core/pkg/evaluator/fractional.go:30
flagd                    | github.com/diegoholiveira/jsonlogic/v3.operation
flagd                    | 	/go/pkg/mod/github.com/diegoholiveira/jsonlogic/[email protected]/operation.go:15
flagd                    | github.com/diegoholiveira/jsonlogic/v3.apply
flagd                    | 	/go/pkg/mod/github.com/diegoholiveira/jsonlogic/[email protected]/jsonlogic.go:493
flagd                    | github.com/diegoholiveira/jsonlogic/v3.ApplyInterface
flagd                    | 	/go/pkg/mod/github.com/diegoholiveira/jsonlogic/[email protected]/jsonlogic.go:587
flagd                    | github.com/diegoholiveira/jsonlogic/v3.Apply
flagd                    | 	/go/pkg/mod/github.com/diegoholiveira/jsonlogic/[email protected]/jsonlogic.go:522
flagd                    | github.com/open-feature/flagd/core/pkg/evaluator.(*JSON).evaluateVariant
flagd                    | 	/src/core/pkg/evaluator/json.go:339
flagd                    | github.com/open-feature/flagd/core/pkg/evaluator.resolve[...]
flagd                    | 	/src/core/pkg/evaluator/json.go:276
flagd                    | github.com/open-feature/flagd/core/pkg/evaluator.(*JSON).ResolveAllValues
flagd                    | 	/src/core/pkg/evaluator/json.go:159
flagd                    | github.com/open-feature/flagd/core/pkg/service/flag-evaluation.(*FlagEvaluationService).ResolveAll
flagd                    | 	/src/core/pkg/service/flag-evaluation/flag_evaluator_v2.go:65
flagd                    | connectrpc.com/connect.NewUnaryHandler[...].func1
flagd                    | 	/go/pkg/mod/connectrpc.com/[email protected]/handler.go:52
flagd                    | connectrpc.com/connect.NewUnaryHandler[...].func2
flagd                    | 	/go/pkg/mod/connectrpc.com/[email protected]/handler.go:84
flagd                    | connectrpc.com/connect.(*Handler).ServeHTTP
flagd                    | 	/go/pkg/mod/connectrpc.com/[email protected]/handler.go:265
flagd                    | buf.build/gen/go/open-feature/flagd/connectrpc/go/flagd/evaluation/v1/evaluationv1connect.NewServiceHandler.func1
flagd                    | 	/go/pkg/mod/buf.build/gen/go/open-feature/flagd/connectrpc/[email protected]/flagd/evaluation/v1/evaluationv1connect/evaluation.connect.go:245
flagd                    | net/http.HandlerFunc.ServeHTTP
flagd                    | 	/usr/local/go/src/net/http/server.go:2166
flagd                    | github.com/open-feature/flagd/core/pkg/service/flag-evaluation.bufSwitchHandler.ServeHTTP
flagd                    | 	/src/core/pkg/service/flag-evaluation/connect_service.go:51
flagd                    | github.com/open-feature/flagd/core/pkg/service/flag-evaluation.(*ConnectService).setupServer.(*ConnectService).AddMiddleware.Middleware.Handler.func1.1
flagd                    | 	/src/core/pkg/service/middleware/metrics/http_metrics.go:105
flagd                    | github.com/open-feature/flagd/core/pkg/service/middleware/metrics.Middleware.Measure
flagd                    | 	/src/core/pkg/service/middleware/metrics/http_metrics.go:90
flagd                    | github.com/open-feature/flagd/core/pkg/service/flag-evaluation.(*ConnectService).setupServer.(*ConnectService).AddMiddleware.Middleware.Handler.func1
flagd                    | 	/src/core/pkg/service/middleware/metrics/http_metrics.go:104
flagd                    | net/http.HandlerFunc.ServeHTTP
flagd                    | 	/usr/local/go/src/net/http/server.go:2166
flagd                    | github.com/open-feature/flagd/core/pkg/service/flag-evaluation.(*ConnectService).setupServer.(*ConnectService).AddMiddleware.Middleware.Handler.(*Cors).Handler.func2
flagd                    | 	/go/pkg/mod/github.com/rs/[email protected]/cors.go:281
flagd                    | net/http.HandlerFunc.ServeHTTP
flagd                    | 	/usr/local/go/src/net/http/server.go:2166
flagd                    | golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP
flagd                    | 	/go/pkg/mod/golang.org/x/[email protected]/http2/h2c/h2c.go:125
flagd                    | net/http.serverHandler.ServeHTTP
flagd                    | 	/usr/local/go/src/net/http/server.go:3137
flagd                    | net/http.(*conn).serve
flagd                    | 	/usr/local/go/src/net/http/server.go:2039

Hmmm, only fractional is the adServiceFailure. the session span attribute is missing. I don't think I1m touching those requests in my PR, I'll dig around if I can reproduce the issue. EDIT: I see the issue too, digging further

@klucsik
Copy link
Contributor Author

klucsik commented Apr 10, 2024

The session var should be setted here:

client.setEvaluationContext(new MutableContext().add("session", uuid.toString()));

This is very far away from my changes, testing main now

@klucsik
Copy link
Contributor Author

klucsik commented Apr 10, 2024

main works fine, but my PR somehow introduces these errors, I`ll investigate more

@klucsik klucsik marked this pull request as draft April 10, 2024 13:59
@klucsik
Copy link
Contributor Author

klucsik commented Apr 16, 2024

These changes are working as expected, I find no problem in services behavior. But these error messages are emitted every time the react client makes a request against flagd. My suspect would be the interaction between https://github.com/open-feature/js-sdk and flagd.

@klucsik
Copy link
Contributor Author

klucsik commented Apr 16, 2024

The problem will be around this caching: https://github.com/open-feature/js-sdk-contrib/tree/main/libs/providers/flagd-web#caching
The provider requesting all featureflags, and does not provide any context, and flagd throws errors on this

Copy link
Member

@austinlparker austinlparker left a comment

Choose a reason for hiding this comment

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

sgtm - are we blocked on the provider being weird?

@klucsik
Copy link
Contributor Author

klucsik commented May 2, 2024

at provider side, this open-feature/flagd#1295 solves the issue, we had in logs.
And as I see, this fix is out in the wild now: https://github.com/open-feature/flagd/releases/tag/core%2Fv0.9.1 and this PR bump flagd version to 0.10.1, so we are not bloked

Copy link

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

klucsik and others added 2 commits May 10, 2024 13:26
Signed-off-by: Michael Beemer <[email protected]>
@julianocosta89 julianocosta89 merged commit f909932 into open-telemetry:main May 13, 2024
29 checks passed
@klucsik klucsik deleted the slowloading-images branch May 13, 2024 09:27
AlexPSplunk pushed a commit to splunk/edu-opentelemetry-demo that referenced this pull request Jul 10, 2024
* [frontend] Add FeatureFlag Provider to client side, add http fault filter to envoy, add slowloading featureflag

* refactor web sdk usage

Signed-off-by: Michael Beemer <[email protected]>

---------

Signed-off-by: Michael Beemer <[email protected]>
Co-authored-by: Michael Beemer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-update-required Requires documentation update 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.

4 participants