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.

Sign up for GitHub

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

[ProductCatalogService] Add product catalog service problem patterns #1382

Closed
wants to merge 6 commits into from
Closed

[ProductCatalogService] Add product catalog service problem patterns #1382

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4592242
[product-catalog-service] refactor feature flag check to be reusable
klucsik Feb 15, 2024
0a93be9
[product-catalog-service] Add ItemNotFound and ItemListInternal probl…
klucsik Feb 15, 2024
70bc2fd
Add changelog
klucsik Feb 15, 2024
600fc5a
Update src/ffspostgres/init-scripts/20-ffs_data.sql
klucsik Feb 15, 2024
f898f6c
Merge branch 'main' into klucsik-add-product-catalog-service-problem-…
julianocosta89 Feb 20, 2024
96969b6
rename flags to be more coherent
klucsik Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ the release.
([#1377](https://github.com/open-telemetry/opentelemetry-demo/pull/1377))
* [cartservice] update .NET to .NET 8.0.2
([#1380](https://github.com/open-telemetry/opentelemetry-demo/pull/1380))
* [productcatalogservice] Added Problems to ListProducts and GetProduct endpoints
([#1382](https://github.com/open-telemetry/opentelemetry-demo/pull/1382))

## 1.7.2

Expand Down
4 changes: 3 additions & 1 deletion src/ffspostgres/init-scripts/20-ffs_data.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@ VALUES
('productCatalogFailure', 'Fail product catalog service on a specific product', 0),
('recommendationCache', 'Cache recommendations', 0),
('adServiceFailure', 'Fail ad service requests', 0),
('cartServiceFailure', 'Fail cart service requests', 0);
('cartServiceFailure', 'Fail cart service requests', 0)
('ItemNotFound', 'Fail Product Catalog Get - Not Found', 0),
('ItemListInternal', 'Fail Product Catalog List - Internal server error', 0);
20 changes: 19 additions & 1 deletion src/productcatalogservice/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,13 @@ func (p *productCatalog) Watch(req *healthpb.HealthCheckRequest, ws healthpb.Hea
func (p *productCatalog) ListProducts(ctx context.Context, req *pb.Empty) (*pb.ListProductsResponse, error) {
span := trace.SpanFromContext(ctx)

if p.getFeatureFlag(ctx, "ItemListInternal") {
Copy link
Member

Choose a reason for hiding this comment

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

How would a 500 error happen here naturally? Is there a better way to represent this failure scenario?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This 500 error is initially meant to be in the SearchProducts endpoint, but that is not called by anything, thats why I moved this to here to be able to throw an Internal error in a Go service, so we can demonstrate the detection of that.

msg := fmt.Sprintf("ItemListInternal flag enabled! List failed!")
span.SetStatus(otelcodes.Error, msg)
span.AddEvent(msg)
return nil, status.Errorf(codes.Internal, msg)
}

span.SetAttributes(
attribute.Int("app.products.count", len(catalog)),
)
Expand All @@ -237,6 +244,13 @@ func (p *productCatalog) GetProduct(ctx context.Context, req *pb.GetProductReque
return nil, status.Errorf(codes.Internal, msg)
}

if p.getFeatureFlag(ctx, "ItemNotFound") {
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit confused by this failure case -- do we need to force it via flag? Wouldn't simply passing an invalid product ID to the service trigger the same result as this?

Copy link
Member

Choose a reason for hiding this comment

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

The flag would still need to be in place to control when to send the wrong id, but yeah, agree with you.

Copy link
Member

Choose a reason for hiding this comment

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

Hmmmm, validating the PR I actually understood the scenario.
ItemNotFound is being triggered just when navigating to the product page.

I can see that being used in demos, to showcase errors on a page that is part of the checkout flow.

ItemListInternal on the other hand, fails on listing the products, so you can't even see the products.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could move the ItemNotFound logic to loadgenerator to query some clearly invalid products, the requirement from our end is to demonstrate a notfound error in the sample

msg := fmt.Sprintf("ItemNotFound flag enabled! Product Not Found!")
span.SetStatus(otelcodes.Error, msg)
span.AddEvent(msg)
return nil, status.Errorf(codes.NotFound, msg)
}

var found *pb.Product
for _, product := range catalog {
if req.Id == product.Id {
Expand Down Expand Up @@ -281,6 +295,11 @@ func (p *productCatalog) checkProductFailure(ctx context.Context, id string) boo
return false
}

return p.getFeatureFlag(ctx, "productCatalogFailure")
}

func (p *productCatalog) getFeatureFlag(ctx context.Context, flagName string) bool {

conn, err := createClient(ctx, p.featureFlagSvcAddr)
if err != nil {
span := trace.SpanFromContext(ctx)
Expand All @@ -289,7 +308,6 @@ func (p *productCatalog) checkProductFailure(ctx context.Context, id string) boo
}
defer conn.Close()

flagName := "productCatalogFailure"
ffResponse, err := pb.NewFeatureFlagServiceClient(conn).GetFlag(ctx, &pb.GetFlagRequest{
Name: flagName,
})
Expand Down