feat: Create interface for eval events.

[connyay]

This PR

Before this change the eval services used a unexported
struct which prevented creating eval services outside of
this package.

This change creates a new IEvents interface that allows
providing custom impls of flag eval services.

Related Issues

Notes

Follow-up Tasks

How to test

[netlify]

✅ Deploy Preview for polite-licorice-3db33c canceled.

Name	Link
🔨 Latest commit	b38764d
🔍 Latest deploy log	https://app.netlify.com/sites/polite-licorice-3db33c/deploys/6628199b491e7c0008d7566f

[connyay]

would like to mount an eval service in an existing server, but don't see a way I can get this working without EventingConfiguration being exported.

It is possible I am breaking the warranty seal here, but this is pseudo code of what I'm trying to do:

func (f *flagdWrapper) Init(ctx context.Context) error {
	log := logging.GetLogger().With("component", "flagd.evaluation.v1").Desugar()
	reqIDLogging := false
	logger := logger.NewLogger(log, reqIDLogging)
	marshalOpts := flagdService.WithJSON(
		protojson.MarshalOptions{EmitUnpopulated: true},
		protojson.UnmarshalOptions{DiscardUnknown: true},
	)
	var (
		eval            evaluator.IEvaluator
		metricsRecorder *telemetry.MetricsRecorder
	)
	evalSvc := flagdService.NewFlagEvaluationService(
		logger, eval, nil /*eventingCfg*/, metricsRecorder,
	)
	_, flagdHandler := evaluationV1.NewServiceHandler(evalSvc, marshalOpts)

	prefix := "/flagd.evaluation.v1.Service/"
	service.MustGetHTTPServeMux(ctx).
		Handle(prefix, flagdHandler)

	return f.Initializer.Init(ctx)
}

[Kavindu-Dodan]

would like to mount an eval service in an existing server, but don't see a way I can get this working without EventingConfiguration being exported.

It is possible I am breaking the warranty seal here, but this is pseudo code of what I'm trying to do:

func (f *flagdWrapper) Init(ctx context.Context) error {
	log := logging.GetLogger().With("component", "flagd.evaluation.v1").Desugar()
	reqIDLogging := false
	logger := logger.NewLogger(log, reqIDLogging)
	marshalOpts := flagdService.WithJSON(
		protojson.MarshalOptions{EmitUnpopulated: true},
		protojson.UnmarshalOptions{DiscardUnknown: true},
	)
	var (
		eval            evaluator.IEvaluator
		metricsRecorder *telemetry.MetricsRecorder
	)
	evalSvc := flagdService.NewFlagEvaluationService(
		logger, eval, nil /*eventingCfg*/, metricsRecorder,
	)
	_, flagdHandler := evaluationV1.NewServiceHandler(evalSvc, marshalOpts)

	prefix := "/flagd.evaluation.v1.Service/"
	service.MustGetHTTPServeMux(ctx).
		Handle(prefix, flagdHandler)

	return f.Initializer.Init(ctx)
}

Thank you for showing the intended usage of the service. I proposed a few improvements to the PR so that it improves testability and makes component interactions clearer.

However, you should understand that as maintainers we cannot guarantee backward compatibility of the ConnectService in the long run. On the other hand, we will continue to avoid breaking changes in the flagd core [1] as this is intended to be a reusable component. For example, flagd Go provider use flagd core internally for in-process evaluations.

[1] - https://github.com/open-feature/flagd/tree/main/core

[Kavindu-Dodan]

@connyay let me know if you can work on the proposed changes. If so we can include this alogn with our next release - #1292

[connyay]

@connyay let me know if you can work on the proposed changes. If so we can include this alogn with our next release - #1292

I can rework this to use interfaces. Will get to it over the next few days. You dont need to wait on me to cut a release though.

[Kavindu-Dodan]

@connyay let me know if you can work on the proposed changes. If so we can include this alogn with our next release - #1292

I can rework this to use interfaces. Will get to it over the next few days. You dont need to wait on me to cut a release though.

Thanks and look forward to the change.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.25%. Comparing base (1c530ab) to head (b38764d).
Report is 56 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1288      +/-   ##
==========================================
+ Coverage   73.69%   77.25%   +3.56%     
==========================================
  Files          32       20      -12     
  Lines        3140     1618    -1522     
==========================================
- Hits         2314     1250    -1064     
+ Misses        717      286     -431     
+ Partials      109       82      -27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Kavindu-Dodan]

Changes looks good 🎉

@connyay could you please rebase and add DCO signature to commits. You can check the guide here - https://github.com/open-feature/flagd/pull/1288/checks?check_run_id=24131045370

[connyay]

However, you should understand that as maintainers we cannot guarantee backward compatibility of the ConnectService in the long run. On the other hand, we will continue to avoid breaking changes in the flagd core [1] as this is intended to be a reusable component. For example, flagd Go provider use flagd core internally for in-process evaluations.

Totally understand. Appreciate you still being open to this PR even with that being said 👍

[Kavindu-Dodan]

However, you should understand that as maintainers we cannot guarantee backward compatibility of the ConnectService in the long run. On the other hand, we will continue to avoid breaking changes in the flagd core [1] as this is intended to be a reusable component. For example, flagd Go provider use flagd core internally for in-process evaluations.

Totally understand. Appreciate you still being open to this PR even with that being said 👍

👍 you're welcome. Please continuously check flagd releases for any important changes.

I will merge this when other maintainers approve

[toddbaert]

I'm fine with this, given the caveats @Kavindu-Dodan mentioned.