333 Telemetry collection is configurable down to the Python module level

:thinking: I picked a bad first example, it carries too much incidental baggage. Here’s a better example in a simpler context: logger.info(f"Added new graph with id {graph_id} to memory.").

class GraphMemory(LMMemory):
    # ...
        def _build_graph(self, locations, features, graph_id, input_channel):
        # ...
        logger.info(f"Added new graph with id {graph_id} to memory.")
        # Note: duplication, not replacement
        # I don't know what's the most ergonomic API yet.
        # Creating a new class for every telemetry line feels bad.
        # I think snapshots warrant a class per snapshot due to their
        # nature, but events feel more like a bag of properties
        telemeter.info("new graph added", {
            "tbp.monty.learning_module.graph.id": graph_id
        })
        # or something like...
        telemeter.info(events.NewGraphAdded(graph_id))
        # ...

For config, it should be exactly like New Logging Configuration Options (unless it shouldn’t). The only difference would be two new sibling keys to logging: telemetry and snapshots, with only telemetry for now.

experiment:
  config:
    ...
    logging:
      ...
      loggers:
        # Turn logging on for all of the MuJoCo modules
        tbp.monty.simulators.mujoco:
          level: DEBUG
        # Enable some motor policy logging
        tbp.monty.frameworks.models.motor_policies:
          level: DEBUG
    telemetry:
      ...
      loggers:
        telemetry.tbp.monty.some.module:
          level: DEBUG
    snapshots:
      ...
      loggers:
        # Turn off all snapshots
        snapshots.tbp.monty:
          level: CRITICAL
        # Turn on one specific module
        snapshots.tbp.monty.some.module:
          level: DEBUG

I just now noticed that Python doesn’t have a native TRACE log level, so we don’t need to add one. DEBUG will be fine for snapshots. The snapshots. namespace prefix is what gives us snapshot-specific control. Again, just sharing this for illustration purposes, we don’t need snapshots in the first PR.

1 Like