-
Notifications
You must be signed in to change notification settings - Fork 673
NOISSUE - Group service middleware into single folder #2472
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
Conversation
be16129 to
8128e0d
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2472 +/- ##
==========================================
+ Coverage 34.49% 43.66% +9.17%
==========================================
Files 318 211 -107
Lines 46477 33458 -13019
==========================================
- Hits 16030 14608 -1422
+ Misses 29651 18107 -11544
+ Partials 796 743 -53 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
430d8c0 to
5de0889
Compare
5de0889 to
655b913
Compare
f7a9075 to
49da8ce
Compare
|
@felixgateru Please resolve conflicts. |
49da8ce to
dd77778
Compare
dd77778 to
c4c6771
Compare
c4c6771 to
6fc2d52
Compare
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
6fc2d52 to
eef1496
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also rename factory functions to NewLogging, NewMetrics... so we have middleware.New<middleware type>.
clients/middleware/doc.go
Outdated
| // Package middleware provides authorization, logging, metrics and tracing middleware | ||
| // for Magistrala Clients Service. | ||
| // | ||
| // For more details about tracing instrumentation for Magistrala messaging refer | ||
| // to the documentation at https://docs.magistrala.abstractmachines.fr/tracing/. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is SuperMQ, not Magistrala.
| // Package middleware provides tracing, logging and metrics middleware | ||
| // for Magistrala Notifiers service. | ||
| // | ||
| // For more details about tracing instrumentation for Magistrala messaging refer | ||
| // to the documentation at https://docs.magistrala.abstractmachines.fr/tracing/. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, SMQ, not MG.
provision/middleware/doc.go
Outdated
| // Package middleware provides logging middleware for Magistrala | ||
| // Provision service. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same. Check other MG references as well.
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
| logger *slog.Logger | ||
| } | ||
|
|
||
| // NewRoleManagerLoggingMiddleware adds logging facilities to the core service. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the same approach: NewLogging (and NewAuth snd NewMetric) here, so we have something like middlewarealias.NewLogging....
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
channels/middleware/authorization.go
Outdated
| return nil, err | ||
| } | ||
| ram, err := rmMW.NewRoleManagerAuthorizationMiddleware(policies.ChannelType, svc, authz, rolesOpPerm, callout) | ||
| ram, err := rmMW.NewAuthorization(policies.ChannelType, svc, authz, rolesOpPerm, callout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using mixedCaps in package aliases. Use rolemw here.
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
What type of PR is this?
This is a refactor as it groups current service middleware.
What does this do?
This pr creates the following folder structure for magistrala services:
service--
middleware----
authorization.go----
logging.go----
metrics.go----
tracing.go----
doc.goWhich issue(s) does this PR fix/relate to?
Noissue
Have you included tests for your changes?
No
Did you document any new/modified feature?
Yes changes include godoc documentation
Notes
To be merged after #2444