feat: Update the go feature server from Expedia code repo. #4665

shuchu · 2024-10-22T16:47:39Z

What this PR does / why we need it:

Update the improved Go feature server code from ExpediaGroup/Feast to our code repo.

Which issue(s) this PR fixes:

shuchu · 2024-10-22T16:49:51Z

shuchu · 2024-10-23T03:04:09Z

[Update 10-22-2024] Now it can compile, build and build docker image. All configures in the Makefile under the "#Go SDK & embedded" are working, except the "install-go-ci-dependencies".

The current "install-go-ci-dependencies" is obsoleted. We need to fix it when we setup the integration test for Go feature server.

!!Warning, the current version my not function correctly. need more testing and investigating.

shuchu · 2024-11-01T03:53:59Z

[Update 10-31-2024]

"make test-go" works now
DataDog related observability instrumentation code were commented out but not removed. I keep these code and plan to use OTEL to replace them.

EXPEbdodla · 2024-11-01T16:09:21Z

go/internal/feast/registry/http.go

@@ -0,0 +1,212 @@
+package registry


This needs to be switched to remote registry. We built http registry to overcome the password sharing for SQL Registry. HTTP Registry which is FastAPI based is not contributed to Feast. We are also planning to leverage Remote registry and move over from HTTP one.

Thank you for the info, @EXPEbdodla ! Let me try to resolve this.

I have removed the code of HTTP Registry. And now, only the File based Registry is supported.

shuchu · 2024-11-05T03:27:04Z

[Update 11/04/2024]

The HTTP Registry code is removed as it depends on an external HTTP server as @EXPEbdodla pointed out.
The default Registry is File based one now.

shuchu · 2024-11-08T03:26:52Z

go/internal/feast/featurestore.go

+	}
+	sanitizedProjectName := strings.Replace(config.Project, "_", "-", -1)
+	productName := os.Getenv("PRODUCT")
+	endpoint := fmt.Sprintf("%s-transformations.%s.svc.cluster.local:80", sanitizedProjectName, productName)


Is this an external Transformation server? I guess we need to remove this and related code? @EXPEbdodla

Initially transformations are running with GOPY bindings. That's not working at scale. So we implemented it similar to Java Feature Server.

Thank you, @EXPEbdodla I guess I need to work on this part.

I chose an "easy" fix for this. 😿

Yes. We should get the transformation server endpoint from feature_store.yaml.

Signed-off-by: Shuchu Han <[email protected]>

…ntation code. Signed-off-by: Shuchu Han <[email protected]>

Signed-off-by: Shuchu Han <[email protected]>

…point. Signed-off-by: Shuchu Han <[email protected]>

shuchu · 2024-11-21T04:43:06Z

[Update 11-20-2024]
1, about the transformation service endpoint issue, I couldn't find a better solution now since this is a design choice between the "not working" callback function and using Python based transformation gRPC service. As for now, I use a general string to represent the endpoint. Sorry about this. I will spend more time to understand the code and see if we can do it in a better way.
cc: @EXPEbdodla @tokoko @franciscojavierarceo

HaoXuAI · 2024-11-21T06:30:13Z

go/internal/feast/featurestore.go

@@ -297,6 +315,10 @@ func (fs *FeatureStore) readFromOnlineStore(ctx context.Context, entityRows []*p
 	requestedFeatureViewNames []string,
 	requestedFeatureNames []string,
 ) ([][]onlinestore.FeatureData, error) {
+	// Create a Datadog span from context


I think these can be removed or updated as datadog is not part of feast

Yea. Datadog needs to be replaced with OTEL.

yes, I will replace them with OTEL after this PR get merged.

HaoXuAI · 2024-11-21T06:31:02Z

go/internal/feast/onlinestore/redisonlinestore.go

 			}
 		}
 	}

-	if t == redisNode {
+	// Metrics are not showing up when the service name is set to DD_SERVICE


similar here

HaoXuAI · 2024-11-21T06:32:52Z

go/README.md

@@ -1,109 +1,12 @@
-This directory contains the Go logic that's executed by the `EmbeddedOnlineFeatureServer` from Python.


Any reason we are removing the document here?

That piece of code is not needed if we remove GOPY bindings.

shuchu requested review from tokoko and franciscojavierarceo October 22, 2024 16:47

shuchu changed the title ~~feat: Update the go feature server from Expedia code repo.~~ [WIP] feat: Update the go feature server from Expedia code repo. Oct 22, 2024

shuchu force-pushed the feature/migrate-expedia-go-fs branch from 80285e6 to d371247 Compare October 23, 2024 02:43

shuchu force-pushed the feature/migrate-expedia-go-fs branch from 9ee4605 to c5aaf98 Compare October 31, 2024 15:12

shuchu changed the title ~~[WIP] feat: Update the go feature server from Expedia code repo.~~ feat: Update the go feature server from Expedia code repo. Nov 1, 2024

shuchu marked this pull request as ready for review November 1, 2024 03:55

shuchu requested a review from a team as a code owner November 1, 2024 03:55

shuchu requested a review from HaoXuAI November 1, 2024 04:08

EXPEbdodla reviewed Nov 1, 2024

View reviewed changes

shuchu commented Nov 8, 2024

View reviewed changes

shuchu marked this pull request as draft November 11, 2024 03:06

shuchu added 8 commits November 20, 2024 23:15

feat: Update the go feature server from Expedia code repo.

df2f0a6

Signed-off-by: Shuchu Han <[email protected]>

fix: Add go package definition to RegistryServer and Grpcserver.

be909c3

Signed-off-by: Shuchu Han <[email protected]>

fix: Fix the make build-go

23a8156

Signed-off-by: Shuchu Han <[email protected]>

fix: Fix makefile to make test-go work.

37b7266

Signed-off-by: Shuchu Han <[email protected]>

fix: Removed and commented out DataDog related observability instrume…

20aefa9

…ntation code. Signed-off-by: Shuchu Han <[email protected]>

doc: Update the README to mention the contirbution from Expedia Group.

b9eff42

Signed-off-by: Shuchu Han <[email protected]>

fix: Remove the HTTP based Registry.

4eaa6c7

Signed-off-by: Shuchu Han <[email protected]>

fix: Use a general string to represent the transformation service end…

953d62e

…point. Signed-off-by: Shuchu Han <[email protected]>

shuchu force-pushed the feature/migrate-expedia-go-fs branch from b65a6a4 to 953d62e Compare November 21, 2024 04:31

shuchu marked this pull request as ready for review November 21, 2024 04:43

HaoXuAI reviewed Nov 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Update the go feature server from Expedia code repo. #4665

feat: Update the go feature server from Expedia code repo. #4665

shuchu commented Oct 22, 2024 •

edited

Loading

shuchu commented Oct 22, 2024

shuchu commented Oct 23, 2024

shuchu commented Nov 1, 2024

EXPEbdodla Nov 1, 2024

shuchu Nov 2, 2024 •

edited

Loading

shuchu Nov 5, 2024 •

edited

Loading

shuchu commented Nov 5, 2024

shuchu Nov 8, 2024

EXPEbdodla Nov 8, 2024

shuchu Nov 11, 2024

shuchu Nov 21, 2024

EXPEbdodla Nov 22, 2024

shuchu commented Nov 21, 2024

HaoXuAI Nov 21, 2024

EXPEbdodla Nov 22, 2024

shuchu Nov 23, 2024

HaoXuAI Nov 21, 2024

HaoXuAI Nov 21, 2024

EXPEbdodla Nov 22, 2024

		@@ -1,109 +1,12 @@
		This directory contains the Go logic that's executed by the `EmbeddedOnlineFeatureServer` from Python.

feat: Update the go feature server from Expedia code repo. #4665

Are you sure you want to change the base?

feat: Update the go feature server from Expedia code repo. #4665

Conversation

shuchu commented Oct 22, 2024 • edited Loading

What this PR does / why we need it:

Which issue(s) this PR fixes:

shuchu commented Oct 22, 2024

shuchu commented Oct 23, 2024

shuchu commented Nov 1, 2024

Choose a reason for hiding this comment

shuchu Nov 2, 2024 • edited Loading

Choose a reason for hiding this comment

shuchu Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

shuchu commented Nov 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shuchu commented Nov 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shuchu commented Oct 22, 2024 •

edited

Loading

shuchu Nov 2, 2024 •

edited

Loading

shuchu Nov 5, 2024 •

edited

Loading