Skip to content

Commit 88e8076

Browse files
committed
Strawman: Have FTDC try calling DoCommand with {"command": "stats"} to get modular component stats.
1 parent b377bc2 commit 88e8076

File tree

6 files changed

+117
-9
lines changed

6 files changed

+117
-9
lines changed

components/motor/client.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,3 +172,12 @@ func (c *client) IsMoving(ctx context.Context) (bool, error) {
172172
}
173173
return resp.IsMoving, nil
174174
}
175+
176+
func (c *client) Stats() any {
177+
resp, err := c.DoCommand(context.Background(), map[string]any{"command": "stats"})
178+
if err != nil {
179+
return nil
180+
}
181+
182+
return resp
183+
}

ftdc/ftdc.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,9 @@ func (ftdc *FTDC) constructDatum() datum {
316316

317317
for idx := range statsers {
318318
namedStatser := &statsers[idx]
319-
datum.Data[namedStatser.name] = namedStatser.statser.Stats()
319+
if stats := namedStatser.statser.Stats(); stats != nil {
320+
datum.Data[namedStatser.name] = stats
321+
}
320322
}
321323

322324
return datum
@@ -521,6 +523,7 @@ func (ftdc *FTDC) getWriter() (io.Writer, error) {
521523
// to return an error.
522524
os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o644)
523525
if err == nil {
526+
ftdc.logger.Debugw("FTDC created new output file", "path", ftdc.currOutputFile.Name())
524527
break
525528
}
526529
ftdc.logger.Warnw("FTDC failed to open file", "err", err)
@@ -627,7 +630,7 @@ func (ftdc *FTDC) checkAndDeleteOldFiles() error {
627630
// deletion testing. Filename generation uses padding such that we can rely on there before 2/4
628631
// digits for every numeric value.
629632
//
630-
//nolint
633+
// nolint
631634
// Example filename: countingBytesTest1228324349/viam-server-2024-11-18T20-37-01Z.ftdc
632635
var filenameTimeRe = regexp.MustCompile(`viam-server-(\d{4})-(\d{2})-(\d{2})T(\d{2})-(\d{2})-(\d{2})Z.ftdc`)
633636

module/testmodule/main.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ var _ motor.Motor = &testMotor{}
293293

294294
// SetPower trivially implements motor.Motor.
295295
func (tm *testMotor) SetPower(_ context.Context, _ float64, _ map[string]interface{}) error {
296-
return nil
296+
return errors.New("Cannot set power")
297297
}
298298

299299
// GoFor trivially implements motor.Motor.
@@ -336,10 +336,15 @@ func (tm *testMotor) IsPowered(_ context.Context, _ map[string]interface{}) (boo
336336
return false, 0.0, nil
337337
}
338338

339-
// DoCommand trivially implements motor.Motor.
340-
func (tm *testMotor) DoCommand(_ context.Context, _ map[string]interface{}) (map[string]interface{}, error) {
341-
//nolint:nilnil
342-
return nil, nil
339+
func (tm *testMotor) DoCommand(ctx context.Context, req map[string]interface{}) (map[string]interface{}, error) {
340+
if req["command"] == "stats" {
341+
return map[string]any{
342+
"BadSetPowers": 5,
343+
"BadStat": "boo",
344+
}, nil
345+
}
346+
347+
return nil, fmt.Errorf("Unknown command: %v", req["command"])
343348
}
344349

345350
// IsMoving trivially implements motor.Motor.

robot/impl/local_robot.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,8 +370,12 @@ func newWithResources(
370370
// - Guarantee that the `rpcServer` is initialized (enough) when the web service is
371371
// constructed to get a valid copy of its stats object (for the schema's sake). Even if
372372
// the web service has not been "started".
373-
ftdcDir := ftdc.DefaultDirectory(utils.ViamDotDir, partID)
374-
ftdcWorker = ftdc.NewWithUploader(ftdcDir, conn, partID, logger.Sublogger("ftdc"))
373+
if rOpts.ftdcWriter != nil {
374+
ftdcWorker = ftdc.NewWithWriter(rOpts.ftdcWriter, logger.Sublogger("ftdc"))
375+
} else {
376+
ftdcDir := ftdc.DefaultDirectory(utils.ViamDotDir, partID)
377+
ftdcWorker = ftdc.NewWithUploader(ftdcDir, conn, partID, logger.Sublogger("ftdc"))
378+
}
375379
if statser, err := sys.NewSelfSysUsageStatser(); err == nil {
376380
ftdcWorker.Add("proc.viam-server", statser)
377381
}

robot/impl/local_robot_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package robotimpl
22

33
import (
4+
"bytes"
45
"context"
56
"crypto/tls"
67
"crypto/x509"
@@ -44,6 +45,7 @@ import (
4445
"go.viam.com/rdk/config"
4546
"go.viam.com/rdk/examples/customresources/apis/gizmoapi"
4647
"go.viam.com/rdk/examples/customresources/apis/summationapi"
48+
"go.viam.com/rdk/ftdc"
4749
rgrpc "go.viam.com/rdk/grpc"
4850
internalcloud "go.viam.com/rdk/internal/cloud"
4951
"go.viam.com/rdk/logging"
@@ -4653,3 +4655,77 @@ func TestListTunnels(t *testing.T) {
46534655
test.That(t, err, test.ShouldBeNil)
46544656
test.That(t, ttes, test.ShouldResemble, trafficTunnelEndpoints)
46554657
}
4658+
4659+
// TestModuleStats asserts that modular resources that respond to a `DoCommand({"command":
4660+
// "stats"})` will be captured into FTDC.
4661+
func TestModuleStats(t *testing.T) {
4662+
ctx := context.Background()
4663+
logger := logging.NewTestLogger(t)
4664+
4665+
testMotor := resource.NewModel("rdk", "test", "motor")
4666+
testPath := rtestutils.BuildTempModule(t, "module/testmodule")
4667+
cfg := &config.Config{
4668+
Modules: []config.Module{
4669+
{
4670+
Name: "mod",
4671+
ExePath: testPath,
4672+
},
4673+
},
4674+
Components: []resource.Config{
4675+
{
4676+
Name: "h",
4677+
Model: testMotor,
4678+
API: motor.API,
4679+
Attributes: rutils.AttributeMap{},
4680+
},
4681+
},
4682+
}
4683+
4684+
// We will capture FTDC data to an in-memory buffer.
4685+
serializedFTDCData := bytes.NewBuffer(nil)
4686+
robot, err := New(ctx, cfg, nil, logger, WithFTDCWriter(serializedFTDCData))
4687+
test.That(t, err, test.ShouldBeNil)
4688+
defer robot.Close(ctx)
4689+
4690+
options, _, addr := robottestutils.CreateBaseOptionsAndListener(t)
4691+
err = robot.StartWeb(ctx, options)
4692+
test.That(t, err, test.ShouldBeNil)
4693+
4694+
// Create a client connection to the robot.
4695+
robotConn, err := rgrpc.Dial(ctx, addr, logger)
4696+
test.That(t, err, test.ShouldBeNil)
4697+
defer utils.UncheckedErrorFunc(robotConn.Close)
4698+
4699+
// Wrap the client connection as a motor connection.
4700+
motorRes, err := motor.NewClientFromConn(ctx, robotConn, "some-remote", motor.Named("h"), logger)
4701+
test.That(t, err, test.ShouldBeNil)
4702+
4703+
// Assert that a call to `motor.client.Stats` results in a valid response.
4704+
resp := motorRes.(ftdc.Statser).Stats()
4705+
test.That(t, resp.(map[string]any)["BadSetPowers"], test.ShouldEqual, 5)
4706+
4707+
// Wait for a few seconds, hoping data gets written to FTDC and stop the robot.
4708+
time.Sleep(3 * time.Second)
4709+
robot.Close(ctx)
4710+
4711+
// Parse the FTDC data.
4712+
parsed, err := ftdc.Parse(serializedFTDCData)
4713+
test.That(t, err, test.ShouldBeNil)
4714+
4715+
// Assert we have some datapoints
4716+
test.That(t, len(parsed), test.ShouldBeGreaterThan, 0)
4717+
4718+
// Iterate over the FTDC data. Count the number of `BadSetPowers` occurrences.
4719+
badSetPowerCnt := 0
4720+
for _, datum := range parsed {
4721+
for _, reading := range datum.Readings {
4722+
if reading.MetricName == "rdk:component:motor/h.ResStats.BadSetPowers" {
4723+
badSetPowerCnt++
4724+
}
4725+
}
4726+
}
4727+
4728+
// We cannot assert that `badSetPowerCnt` ought to be exactly `len(parsed)`. FTDC may accrue
4729+
// data points before the module and resource are registered/created.
4730+
test.That(t, badSetPowerCnt, test.ShouldBeGreaterThan, 0)
4731+
}

robot/impl/robot_options.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package robotimpl
22

33
import (
4+
"io"
5+
46
"go.viam.com/rdk/robot/web"
57
)
68

@@ -22,6 +24,8 @@ type options struct {
2224
// whether or not to run FTDC
2325
enableFTDC bool
2426

27+
ftdcWriter io.Writer
28+
2529
// disableCompleteConfigWorker starts the robot without the complete config worker - should only be used for tests.
2630
disableCompleteConfigWorker bool
2731
}
@@ -55,6 +59,13 @@ func WithFTDC() Option {
5559
})
5660
}
5761

62+
func WithFTDCWriter(writer io.Writer) Option {
63+
return newFuncOption(func(o *options) {
64+
o.enableFTDC = true
65+
o.ftdcWriter = writer
66+
})
67+
}
68+
5869
// WithWebOptions returns a Option which sets the streamConfig
5970
// used to enable audio/video streaming over WebRTC.
6071
func WithWebOptions(opts ...web.Option) Option {

0 commit comments

Comments
 (0)