-
Notifications
You must be signed in to change notification settings - Fork 0
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
SMQ - 2546 - Add telemetry aggregation for clients telemetry #1
base: main
Are you sure you want to change the base?
Changes from all commits
53cb10b
07dbb86
b2e750c
320c8eb
7f3c850
ac60aa8
2e5bd94
1b6377c
e9226c7
457ef9d
0d127da
f1aeccf
db903c4
e300f15
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ import ( | |
jaegerclient "github.com/absmach/supermq/pkg/jaeger" | ||
"github.com/absmach/supermq/pkg/messaging/brokers" | ||
brokerstracing "github.com/absmach/supermq/pkg/messaging/brokers/tracing" | ||
msgevents "github.com/absmach/supermq/pkg/messaging/events" | ||
"github.com/absmach/supermq/pkg/messaging/handler" | ||
mqttpub "github.com/absmach/supermq/pkg/messaging/mqtt" | ||
"github.com/absmach/supermq/pkg/server" | ||
|
@@ -142,6 +143,13 @@ func main() { | |
} | ||
defer mpub.Close() | ||
|
||
mpub, err = msgevents.NewPublisherMiddleware(ctx, mpub, cfg.ESURL) | ||
if err != nil { | ||
logger.Error(fmt.Sprintf("failed to create event store middleware: %s", err)) | ||
exitCode = 1 | ||
return | ||
} | ||
|
||
fwd := mqtt.NewForwarder(brokers.SubjectAllChannels, logger) | ||
fwd = mqtttracing.New(serverConfig, tracer, fwd, brokers.SubjectAllChannels) | ||
if err := fwd.Forward(ctx, svcName, bsub, mpub); err != nil { | ||
|
@@ -159,9 +167,9 @@ func main() { | |
defer np.Close() | ||
np = brokerstracing.NewPublisher(serverConfig, tracer, np) | ||
|
||
es, err := events.NewEventStore(ctx, cfg.ESURL, cfg.Instance) | ||
np, err = msgevents.NewPublisherMiddleware(ctx, np, cfg.ESURL) | ||
if err != nil { | ||
logger.Error(fmt.Sprintf("failed to create %s event store : %s", svcName, err)) | ||
logger.Error(fmt.Sprintf("failed to create event store middleware: %s", err)) | ||
Comment on lines
+170
to
+172
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Revisit closing the final wrapped The same pattern applies here as with |
||
exitCode = 1 | ||
return | ||
} | ||
|
@@ -198,7 +206,15 @@ func main() { | |
defer channelsHandler.Close() | ||
logger.Info("Channels service gRPC client successfully connected to channels gRPC server " + channelsHandler.Secure()) | ||
|
||
h := mqtt.NewHandler(np, es, logger, clientsClient, channelsClient) | ||
h := mqtt.NewHandler(np, logger, clientsClient, channelsClient) | ||
|
||
h, err = events.NewEventStoreMiddleware(ctx, h, cfg.ESURL, cfg.Instance) | ||
if err != nil { | ||
logger.Error(fmt.Sprintf("failed to create event store middleware: %s", err)) | ||
exitCode = 1 | ||
return | ||
} | ||
|
||
h = handler.NewTracing(tracer, h) | ||
|
||
if cfg.SendTelemetry { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,10 @@ import ( | |
"github.com/absmach/supermq/journal" | ||
) | ||
|
||
var _ supermq.Response = (*pageRes)(nil) | ||
var ( | ||
_ supermq.Response = (*pageRes)(nil) | ||
_ supermq.Response = (*clientTelemetryRes)(nil) | ||
) | ||
|
||
type pageRes struct { | ||
journal.JournalsPage `json:",inline"` | ||
|
@@ -31,3 +34,15 @@ func (res pageRes) Empty() bool { | |
type clientTelemetryRes struct { | ||
journal.ClientTelemetry `json:",inline"` | ||
} | ||
|
||
func (res clientTelemetryRes) Headers() map[string]string { | ||
return map[string]string{} | ||
} | ||
|
||
func (res clientTelemetryRes) Code() int { | ||
return http.StatusOK | ||
} | ||
|
||
func (res clientTelemetryRes) Empty() bool { | ||
return false | ||
} | ||
Comment on lines
+46
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Implement proper Empty check for clientTelemetryRes. The func (res clientTelemetryRes) Empty() bool {
- return false
+ return res.ClientTelemetry.Messages == 0 &&
+ len(res.ClientTelemetry.Subscriptions) == 0
}
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,13 +140,21 @@ func (page JournalsPage) MarshalJSON() ([]byte, error) { | |
type ClientTelemetry struct { | ||
ClientID string `json:"client_id"` | ||
DomainID string `json:"domain_id"` | ||
Subscriptions []string `json:"subscriptions"` | ||
Subscriptions uint64 `json:"subscriptions"` | ||
InboundMessages uint64 `json:"inbound_messages"` | ||
OutboundMessages uint64 `json:"outbound_messages"` | ||
FirstSeen time.Time `json:"first_seen"` | ||
LastSeen time.Time `json:"last_seen"` | ||
} | ||
|
||
type ClientSubscription struct { | ||
ID string `json:"id" db:"id"` | ||
SubscriberID string `json:"subscriber_id" db:"subscriber_id"` | ||
ChannelID string `json:"channel_id" db:"channel_id"` | ||
Subtopic string `json:"subtopic" db:"subtopic"` | ||
ClientID string `json:"client_id" db:"client_id"` | ||
} | ||
Comment on lines
+150
to
+156
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding validation for ClientSubscription fields. The Add validation methods: func (cs *ClientSubscription) Validate() error {
if cs.ID == "" {
return errors.New("id cannot be empty")
}
if cs.SubscriberID == "" {
return errors.New("subscriber_id cannot be empty")
}
if cs.ChannelID == "" {
return errors.New("channel_id cannot be empty")
}
return nil
} |
||
|
||
// Service provides access to the journal log service. | ||
// | ||
//go:generate mockery --name Service --output=./mocks --filename service.go --quiet --note "Copyright (c) Abstract Machines" | ||
|
@@ -179,4 +187,19 @@ type Repository interface { | |
|
||
// DeleteClientTelemetry removes telemetry data for a client from the database. | ||
DeleteClientTelemetry(ctx context.Context, clientID, domainID string) error | ||
|
||
// AddSubscription adds a subscription to the client telemetry. | ||
AddSubscription(ctx context.Context, sub ClientSubscription) error | ||
|
||
// CountSubscriptions returns the number of subscriptions for a client. | ||
CountSubscriptions(ctx context.Context, clientID string) (uint64, error) | ||
|
||
// RemoveSubscription removes a subscription from the client telemetry. | ||
RemoveSubscription(ctx context.Context, subscriberID string) error | ||
|
||
// IncrementInboundMessages increments the inbound messages count for a client. | ||
IncrementInboundMessages(ctx context.Context, clientID string) error | ||
|
||
// IncrementOutboundMessages increments the outbound messages count for a client. | ||
IncrementOutboundMessages(ctx context.Context, channelID, subtopic string) error | ||
} |
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.
🛠️ Refactor suggestion
Ensure the final wrapped publisher is properly closed upon exit.
After reassigning
mpub
to the wrapped instance returned bymsgevents.NewPublisherMiddleware
, the previously deferredmpub.Close()
will only close the original publisher object. To avoid potential resource leaks, defer the close of the final wrapped publisher instead: