Add initial Otto bot #3

austinlparker · 2025-04-18T18:21:29Z

Not yet complete, but I wanted to get the initial work pushed.

Copilot

Pull Request Overview

This PR introduces the initial implementation of Otto bot, setting up the core infrastructure including configuration, HTTP server, telemetry, database, module/event handling, and test utilities.

Added core application functionality with modular design
Integrated OpenTelemetry for metrics, tracing, and logging
Provided test helpers and example configuration for easier development and testing

Reviewed Changes

Copilot reviewed 25 out of 28 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
otto/internal/testing.go	Test helpers for initializing the app, database, and repository
otto/internal/telemetry.go	OpenTelemetry integration for metrics, traces, and log bridging
otto/internal/server.go	HTTP webhook server implementation with signature verification
otto/internal/repository.go	SQLite repository implementation and transaction support
otto/internal/modules_test.go	Unit tests for module registration and event dispatching
otto/internal/modules.go	Module interface definitions and global module registry
otto/internal/middleware_test.go	Placeholder for removed command middleware tests
otto/internal/middleware.go	Placeholder for removed command middleware
otto/internal/error.go	Application error handling and logging mechanism
otto/internal/db.go	Shared SQLite database connection setup
otto/internal/config.go	YAML configuration loading, validation, and defaults
otto/internal/commands_test.go	Tests for slash command parsing utility
otto/internal/commands.go	Slash command parsing and tracing utilities; minor iteration bug present
otto/internal/app.go	Main application struct, initialization, and shutdown logic
otto/config.example.yaml	Example configuration file
otto/cmd/otto/main.go	Application entrypoint and graceful shutdown logic
otto/README.md	Project documentation and overview

Files not reviewed (3)

otto/Dockerfile: Language not supported
otto/Makefile: Language not supported
otto/go.mod: Language not supported

Comments suppressed due to low confidence (2)

otto/internal/commands.go:20

The function strings.SplitSeq is not part of the standard library; replace it with strings.Split to correctly split the string.

lines := strings.SplitSeq(body, "\n")

otto/internal/commands.go:21

The for loop iterates over indices rather than values; use 'for _, line := range lines {' to correctly iterate over each line.

for line := range lines {

trask · 2025-04-18T20:45:38Z

otto/.github/workflows/test.yml

I think this needs to be not under otto/

agreed, it likely should ignore the .env file no matter where it's placed, but setting it's path here will only exclude it in the otto/ dir. Which may be the spot Austin's using locally, but someone is bound to put it somewhere else eventually. .env files are notorious exposers of secrets :D

adrielp · 2025-04-21T13:22:11Z

otto/.github/workflows/test.yml

+        cache-dependency-path: otto/go.sum
+
+    - name: Install dependencies
+      run: go mod download


Won't go test -v ./... automatically download the dependencies? Is it necessary to pre-download step?

adrielp · 2025-04-21T13:23:53Z

otto/Dockerfile

+    --mount=type=cache,target=/root/.cache/go-build \
+    cd ./otto && go build -o /out/otto ./cmd/otto
+
+FROM debian:bullseye-slim


Since otto (love the name) is a go app, any objections to converting the second stage to a scratch container? It'll be lighter than bullseye-slim & more security, especially since all we need is the binary.

adrielp · 2025-04-21T13:24:49Z

otto/bin/otto.png

I want 1000 of these stickers, just saying :)

adrielp · 2025-04-21T13:28:33Z

otto/cmd/otto/main.go

+
+import (
+	"context"
+	"log/slog"


This is a personal preference thing: using zap instead of slog. Both are supported by the log bridge though.

edit: I just got to the point of looking at the instrumentation code, so nevermind on the secondary statement I made previously.

adrielp · 2025-04-21T13:29:27Z

otto/Makefile

There's a couple helpers I've found useful that could be added here, like licensing, gci, imports, osv-scanner, and formatting. I'm happy to add them on a next pass unless you want to now.

Does goreleaser do that stuff? I was thinking of switching to it anyway.

AFAIK, not directly. It supports hooks though which can hook into the makefile. It's good to have this stuff in local dev. If locked to 1.24 of go it can be incorporated into the tools directive, else, the older pattern can easily be followed (which I haven't migrated to the new pattern yet).

Tools the OTel collector finds useful are: (and i echo)

## OTEL Collector Specific ones _ "github.com/daixiang0/gci" _ "github.com/google/addlicense" _ "golang.org/x/tools/cmd/goimports" _ "golang.org/x/vuln/cmd/govulncheck" _ "gotest.tools/gotestsum" _ "mvdan.cc/gofumpt" ## Other _ "github.com/google/osv-scanner/v2/cmd/osv-scanner" )

Happy to submit a PR if you want me to add some of this.

adrielp · 2025-04-21T13:32:51Z

otto/config.example.yaml

+web_hook_secret: "your_web_hook_secret"
+port: "8080"
+db_path: "data.db"
+github_token: "" # GitHub personal access token for API requests


Instead of a PAT, we should use oidc through gh app auth (i have a otel collector extension i wrote as reference, pretty easy to implement).

Also, can we separate the normal config from the secret configuration here to ensure better security around the token / webhook secret on the next pass?

adrielp · 2025-04-21T13:40:14Z

otto/modules/oncall.go

+				return
+			case <-ticker.C:
+				if err := o.CheckUnacknowledgedTasks(); err != nil {
+					slog.Error("Error checking unacknowledged tasks", "error", err)


nit: there's inconsistency in the casing of log messages. Some start upper, others don't.

austinlparker · 2025-04-21T16:26:53Z

I've fixed the issues identified in the PR review:

Changed strings.SplitSeq to strings.Split in commands.go
Updated the for loop to correctly use range with values (for _, line := range lines)
Improved the slash command detection logic to handle spaces correctly

All tests are now passing.

austinlparker · 2025-04-21T16:32:42Z

I've addressed several of the PR comments:

Fixed the inconsistent capitalization in log messages - all now start with uppercase letters
Fixed the commands.go issues identified in the PR review (using strings.Split instead of strings.SplitSeq and correctly iterating over lines)
Moved the GitHub workflow file from otto/.github/ to the root .github/ directory

The remaining suggestions about the Docker image, workflow improvements, and configuration can be addressed in future PRs.

austinlparker · 2025-04-21T16:42:16Z

claude forgot to actually commit those changes so i guess they'll be in a different PR

initial commit

184755d

austinlparker requested a review from a team as a code owner April 18, 2025 18:21

austinlparker requested a review from Copilot April 18, 2025 18:21

Copilot AI reviewed Apr 18, 2025

View reviewed changes

austinlparker added 3 commits April 18, 2025 14:23

add gha test jobs

3485448

Update test.yml

adaf981

Update README.md

40cd0f8

trask approved these changes Apr 18, 2025

View reviewed changes

adrielp reviewed Apr 21, 2025

View reviewed changes

otto/bin/otto.png

Copy link

Contributor

adrielp Apr 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want 1000 of these stickers, just saying :)

adrielp reviewed Apr 21, 2025

View reviewed changes

This was referenced Apr 21, 2025

Improve Otto Docker container with scratch base image #4

Open

Add additional Makefile helpers for otto #5

Open

Improve Otto configuration with separate secrets handling #6

Closed

Review CI workflow for Otto - optimize dependency handling #7

Open

austinlparker merged commit 80473ac into open-telemetry:main Apr 21, 2025
1 check passed

austinlparker deleted the ap/otto branch April 21, 2025 16:36

austinlparker restored the ap/otto branch April 21, 2025 16:37

austinlparker mentioned this pull request Apr 21, 2025

Fix log message capitalization and commands.go issues #8

Merged

Add initial Otto bot #3

Add initial Otto bot #3

Uh oh!

Conversation

austinlparker commented Apr 18, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adrielp Apr 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adrielp Apr 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adrielp Apr 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

austinlparker commented Apr 21, 2025

Uh oh!

austinlparker commented Apr 21, 2025

Uh oh!

Uh oh!

austinlparker commented Apr 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adrielp Apr 21, 2025 •

edited

Loading

adrielp Apr 21, 2025 •

edited

Loading

adrielp Apr 21, 2025 •

edited

Loading