Skip to content

Components auto-registration #5062

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

Jump to bottom
Merged
merged 3 commits into from
Aug 25, 2022
Merged

Components auto-registration #5062

merged 3 commits into from
Aug 25, 2022

Conversation

ItalyPaleAle
Copy link
Contributor

@ItalyPaleAle ItalyPaleAle commented Aug 23, 2022 •
edited
Loading

This PR changes the way that the component registry works internally.

Before, each component had to have an import in the cmd/daprd/main.go file and then be added to the relevant registry in that file. With over 100 components, that started to become complex to track too.

Now, each component is automatically registered when a file is placed in cmd/daprd/components, with each component getting its own file. For example, this is the state_in-memory.go file:

package components

import (
	inmemory "github.com/dapr/components-contrib/state/in-memory"
	stateLoader "github.com/dapr/dapr/pkg/components/state"
)

func init() {
	stateLoader.DefaultRegistry.RegisterComponent(inmemory.NewInMemoryStateStore, "in-memory")
}

This PR is needed to simplify (greatly) the life of users who are building Dapr with a custom set of components, either adding new ones or stripping down components. The biggest advantage is a significant reduction in merge conflicts when managing a custom set of components.

PS: A companion PR will be needed in components-contrib. Once that's merged, I will remove the temporary replacement in the go.mod dapr/components-contrib#1997

PPS: Also fixes #4860

@ItalyPaleAle ItalyPaleAle requested review from a team as code owners August 23, 2022 00:16
@ItalyPaleAle ItalyPaleAle mentioned this pull request Aug 23, 2022
Merged
@yaron2
Copy link
Member

yaron2 commented Aug 23, 2022

Pinging @pkedy for review and thoughts as the main author of the current registration mechanism

@ItalyPaleAle ItalyPaleAle force-pushed the component-registry branch 2 times, most recently from 8e60ebb to 23565db Compare August 23, 2022 03:30
@codecov
Copy link

codecov bot commented Aug 23, 2022 •
edited
Loading

Codecov Report

Merging #5062 (26f7395) into master (1e9a44c) will increase coverage by 0.07%.
The diff coverage is 92.62%.

@@            Coverage Diff             @@
##           master    #5062      +/-   ##
==========================================
+ Coverage   65.37%   65.45%   +0.07%     
==========================================
  Files         113      113              
  Lines       12740    12723      -17     
==========================================
- Hits         8329     8328       -1     
+ Misses       3820     3811       -9     
+ Partials      591      584       -7
Impacted Files Coverage Δ
pkg/runtime/options.go 22.22% <12.50%> (+2.22%) ⬆️
pkg/components/nameresolution/registry.go 90.90% <83.33%> (+0.43%) ⬆️
pkg/components/bindings/registry.go 100.00% <100.00%> (+9.09%) ⬆️
pkg/components/configuration/registry.go 100.00% <100.00%> (+9.52%) ⬆️
pkg/components/lock/registry.go 100.00% <100.00%> (ø)
pkg/components/middleware/http/registry.go 90.90% <100.00%> (+7.57%) ⬆️
pkg/components/pubsub/registry.go 100.00% <100.00%> (+9.52%) ⬆️
pkg/components/secretstores/registry.go 100.00% <100.00%> (+9.52%) ⬆️
pkg/components/state/registry.go 100.00% <100.00%> (+9.52%) ⬆️
pkg/runtime/runtime.go 67.74% <100.00%> (-0.20%) ⬇️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@daixiang0
Copy link
Member

I have a thought that could we use a method like a template to generate files in cmd/daprd/components based on config (maybe build config), make whole build like a menu?

@ItalyPaleAle
Copy link
Contributor Author

It’s an option for the future. It does raise questions about what the supportability story would be in that case.

Note that some components have slightly different files, especially bindings (they can have both input and output) and middlewares.

for now, this PR is optimized to allow the people who absolutely need to build their own Dapr with a custom selection of components to do that more easily. Especially if you want to remove a lot of components, doing it without this PR makes it very likely to have a lot of merge conflicts over time.

@daixiang0
Copy link
Member

In this way the dependences still exist in go.mod, how do you fix the "merge conflicts" issue?

@ItalyPaleAle
Copy link
Contributor Author

The merge conflicts would be in the cmd/runtime/main.go. If you remove a lot of dependencies (like a user wants to do right now, removing almost all components), then any chance to the main.go file is likely to cause merge conflicts.

For the go.mod file, users could place their own additional dependencies at the very top or very bottom in a separate section. If they just want to remove components, then there's no problem there.

@artursouza
Copy link
Contributor

LGTM. Pending validation passing.

@artursouza
Copy link
Contributor

/ok-to-test

1 similar comment
@artursouza
Copy link
Contributor

/ok-to-test

@ItalyPaleAle
Copy link
Contributor Author

Something's up with the change. I'll investigate and the E2E test failures tomorrow morning.

@dapr-bot
Copy link
Collaborator

End-to-end tests failed on windows. Please check the build logs

@dapr-bot
Copy link
Collaborator

End-to-end tests failed on linux. Please check the build logs

@dapr-bot
Copy link
Collaborator

End-to-end tests failed on linux. Please check the build logs

@dapr-bot
Copy link
Collaborator

End-to-end tests failed on windows. Please check the build logs

@ItalyPaleAle
Copy link
Contributor Author

@artursouza fixed the issue that was causing tests to fail.

@ItalyPaleAle
Copy link
Contributor Author

/ok-to-test

@ItalyPaleAle
Components auto-registration
c61fd83 
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
@artursouza
Copy link
Contributor

/ok-to-test

@dapr-bot
Copy link
Collaborator

Congrats! All end-to-end tests have passed on linux. Thanks for your contribution!

@dapr-bot
Copy link
Collaborator

Congrats! All end-to-end tests have passed on windows. Thanks for your contribution!

@dapr-bot
Copy link
Collaborator

Congrats! All end-to-end tests have passed on linux. Thanks for your contribution!

@dapr-bot
Copy link
Collaborator

End-to-end tests failed on windows. Please check the build logs

@ItalyPaleAle
Copy link
Contributor Author

@yaron2 @artursouza @pkedy any update on this?

I heard from @mcandeia that this will help his work on pluggable components too

@artursouza artursouza merged commit 29233c6 into dapr:master Aug 25, 2022
@artursouza artursouza added this to the v1.9 milestone Aug 25, 2022
Taction added a commit to Taction/dapr that referenced this pull request Sep 5, 2022
@Taction
Merge branch 'master' into feat_add_cache_for_secret
5d216cc 
* master: (134 commits)
  Record metric for service updates (dapr#5114)
  chore: remove duplicate word in comments (dapr#5099)
  Run e2e tests in previous version of K8s on AKS to see if it's breaking Windows tests (dapr#5104)
  add {namespace} option to set consumerID
  Add tracing passthrough for bindings (dapr#5049)
  Improve component init error logs and metrics (dapr#4975)
  Improvements to E2E test apps (dapr#5042)
  Fix component name for aws dynamodb (dapr#5095)
  Fix perf tests not building (dapr#5096)
  Fix missing components in Metadata API result. (dapr#5052)
  Fixed log message (dapr#5088)
  Disable IPFS binding for now (dapr#5092)
  optimize/concurrency: pubsub concurrency map writes in runtime unit test. (dapr#5086)
  Configuration API response to use dictionaries - based on PR dapr#4695 (dapr#5083)
  correct compile error
  correct checking for actors with same id
  fix type error in print
  fix typo
  add log when actor start time and end time is not expected
  Components auto-registration (dapr#5062)
  ...

Signed-off-by: zhangchao <zchao9100@gmail.com>

# Conflicts:
#	go.mod
#	pkg/grpc/api.go
#	pkg/grpc/api_test.go
#	pkg/http/api.go
#	pkg/runtime/runtime.go
ItalyPaleAle added a commit to ItalyPaleAle/dapr-components-contrib that referenced this pull request Sep 14, 2022
@ItalyPaleAle
Updated docs for creating new components
d577636 
Due to dapr/dapr#5062

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
@ItalyPaleAle ItalyPaleAle mentioned this pull request Sep 14, 2022
Merged
dapr-bot pushed a commit to dapr/components-contrib that referenced this pull request Sep 15, 2022
@ItalyPaleAle @berndverst
Updated docs for creating new components (#2078)
b8931a5 
Due to dapr/dapr#5062

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Co-authored-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
pravinpushkar pushed a commit to pravinpushkar/components-contrib that referenced this pull request Sep 16, 2022
@ItalyPaleAle @berndverst @pravinpushkar
Updated docs for creating new components (dapr#2078)
ddc1aab 
Due to dapr/dapr#5062

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Co-authored-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
shubham1172 pushed a commit to shubham1172/components-contrib that referenced this pull request Sep 20, 2022
@ItalyPaleAle @berndverst @shubham1172
Updated docs for creating new components (dapr#2078)
c7a5c81 
Due to dapr/dapr#5062

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Co-authored-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@shubham1172 shubham1172 mentioned this pull request Oct 21, 2022
Closed
@ambaumann ambaumann mentioned this pull request Nov 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@artursouza artursouza artursouza approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.9
Development

Successfully merging this pull request may close these issues.
5 participants
@ItalyPaleAle @yaron2 @daixiang0 @artursouza @dapr-bot