Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion devtools/pgdump-lite/pgd/db.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion devtools/pgdump-lite/pgd/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion devtools/pgdump-lite/pgd/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions expflag/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ type Flag string
const (
Example Flag = "example"
UnivKeys Flag = "univ-keys"
UikLogs Flag = "uik-logs"
Copy link
Member

Choose a reason for hiding this comment

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

nit: https://go.dev/wiki/CodeReviewComments#initialisms

Suggested change
UikLogs Flag = "uik-logs"
UIKLogs Flag = "uik-logs"

also, I'm wondering if there's a different way to describe this than "logs" and "logs support" since it doesn't have anything to do with application logs, and the table itself only records the last status/result, and maintains no running "log" of events.

Maybe uik_requests for the table, and the expflag I'm less sure, maybe uik-debug?

)

var desc = map[Flag]string{
Example: "An example experimental flag to demonstrate usage.",
UnivKeys: "Universal integration key support.",
UikLogs: "Logs support for universal integration keys.",
}

// AllFlags returns a slice of all experimental flags sorted by name.
Expand Down
11 changes: 11 additions & 0 deletions gadb/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 45 additions & 0 deletions gadb/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions integrationkey/queries.sql
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,27 @@ WHERE
id = $1
FOR UPDATE;

-- name: IntKeyLog :exec
INSERT INTO uik_logs (
integration_key_id,
content_type,
user_agent,
raw_body,
status,
error_message,
received_at
) VALUES (
$1, $2, $3, $4, $5, $6, $7
)
ON CONFLICT (integration_key_id) DO UPDATE
SET
content_type = EXCLUDED.content_type,
user_agent = EXCLUDED.user_agent,
raw_body = EXCLUDED.raw_body,
status = EXCLUDED.status,
error_message = EXCLUDED.error_message,
received_at = EXCLUDED.received_at;
Comment on lines +51 to +70
Copy link
Member

Choose a reason for hiding this comment

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

For testing/consistency, we'll want to use DB time -- if we pass in the received_at time, we'll need to do a separate query to get the current time. Instead, we can just rely on the default of now() and on the conflict update set the value to the current DB time.

Suggested change
-- name: IntKeyLog :exec
INSERT INTO uik_logs (
integration_key_id,
content_type,
user_agent,
raw_body,
status,
error_message,
received_at
) VALUES (
$1, $2, $3, $4, $5, $6, $7
)
ON CONFLICT (integration_key_id) DO UPDATE
SET
content_type = EXCLUDED.content_type,
user_agent = EXCLUDED.user_agent,
raw_body = EXCLUDED.raw_body,
status = EXCLUDED.status,
error_message = EXCLUDED.error_message,
received_at = EXCLUDED.received_at;
-- name: IntKeyLog :exec
INSERT INTO uik_logs (
integration_key_id,
content_type,
user_agent,
raw_body,
status,
error_message
) VALUES (
$1, $2, $3, $4, $5, $6
)
ON CONFLICT (integration_key_id) DO UPDATE
SET
content_type = EXCLUDED.content_type,
user_agent = EXCLUDED.user_agent,
raw_body = EXCLUDED.raw_body,
status = EXCLUDED.status,
error_message = EXCLUDED.error_message,
received_at = now();


-- name: IntKeySetConfig :exec
INSERT INTO uik_config(id, config)
VALUES ($1, $2)
Expand Down
82 changes: 81 additions & 1 deletion integrationkey/uik/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"net/http"
"strings"
"unicode/utf8"

"github.com/expr-lang/expr/vm"
"github.com/google/uuid"
Expand All @@ -33,6 +34,13 @@ type TxAble interface {
BeginTx(ctx context.Context, opts *sql.TxOptions) (*sql.Tx, error)
}

const (
LogStatusSuccess = "success"
LogStatusParseError = "parse_error"
LogStatusExecError = "exec_error"
LogStatusSendError = "send_error"
Comment on lines +38 to +41
Copy link
Member

Choose a reason for hiding this comment

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

maybe these could be error types? since success is sort of on it's own

)

func NewHandler(db TxAble, intStore *integrationkey.Store, aStore *alert.Store, evt *event.Bus) *Handler {
return &Handler{intStore: intStore, db: db, alertStore: aStore, evt: evt}
}
Expand Down Expand Up @@ -88,6 +96,47 @@ func (h *Handler) handleAction(ctx context.Context, act gadb.UIKActionV1) (inser
return didInsertSignals, nil
}

// upsertUikLog will save a log of the most recent request to a universal integration key in the db
func (h *Handler) upsertUikLog(
Copy link
Member

Choose a reason for hiding this comment

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

nit: https://go.dev/wiki/CodeReviewComments#initialisms

Suggested change
func (h *Handler) upsertUikLog(
func (h *Handler) upsertUIKLog(

ctx context.Context,
keyID uuid.UUID,
req *http.Request,
rawBody []byte,
status string,
errMsg string,
Comment on lines +101 to +106
Copy link
Member

Choose a reason for hiding this comment

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

nit: with so many args it may be worth using a struct, but if we use explicit types it could at least prevent mixing up status/err and enforce explicit values

Suggested change
ctx context.Context,
keyID uuid.UUID,
req *http.Request,
rawBody []byte,
status string,
errMsg string,
ctx context.Context,
keyID uuid.UUID,
req *http.Request,
rawBody []byte,
status RequestStatus,
err error,

Alternatively, we could just take err and wrap it appropriately to tell the difference

h.upsertUIKLog(ctx, id, req, body, wrapError(err, ParseError))

and then inspect it within the method

) error {
if !expflag.ContextHas(ctx, expflag.UikLogs) {
return nil
}
const maxSize = 2048
if len(rawBody) > maxSize {
rawBody = rawBody[:maxSize]
}

err := gadb.New(h.db).IntKeyLog(ctx, gadb.IntKeyLogParams{
IntegrationKeyID: keyID,
Status: status,
RawBody: rawBody,
ContentType: sql.NullString{
String: req.Header.Get("Content-Type"),
Valid: req.Header.Get("Content-Type") != "",
},
UserAgent: sql.NullString{
String: req.UserAgent(),
Valid: req.UserAgent() != "",
},
Comment on lines +120 to +127
Copy link
Member

Choose a reason for hiding this comment

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

Should we just make these non-null and allow the empty string? not a huge deal but would simplify things

ErrorMessage: sql.NullString{
String: errMsg,
Valid: errMsg != "",
},
})

if err != nil {
return err
}
return nil
}

// EventNewSignals is an event that is triggered when new signals are generated for a service.
type EventNewSignals struct {
ServiceID uuid.UUID
Expand Down Expand Up @@ -117,11 +166,29 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
}
data, err := io.ReadAll(req.Body)
if errutil.HTTPError(ctx, w, err) {
logErr := h.upsertUikLog(ctx, keyID, req, nil, LogStatusParseError, "read error: "+err.Error())
if logErr != nil {
fmt.Println("failed to log UIK request:", logErr)
}
return
Copy link
Member

Choose a reason for hiding this comment

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

use the logger rather than fmt.Print* so it goes to the right place and format

}

if !utf8.Valid(data) {
logErr := h.upsertUikLog(ctx, keyID, req, nil, LogStatusParseError, "invalid UTF-8")
if logErr != nil {
fmt.Println("failed to log UIK request:", logErr)
}
errutil.HTTPError(ctx, w, validation.NewGenericError("invalid UTF-8 in request body"))
return
}

var body any
err = json.Unmarshal(data, &body)
if errutil.HTTPError(ctx, w, validation.WrapError(err)) {
logErr := h.upsertUikLog(ctx, keyID, req, data, LogStatusParseError, "invalid json: "+err.Error())
if logErr != nil {
fmt.Println("failed to log UIK request:", logErr)
}
return
}

Expand Down Expand Up @@ -156,13 +223,22 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
var vm vm.VM
actions, err := compiled.Run(&vm, env)
if errutil.HTTPError(ctx, w, validation.WrapError(err)) {
logErr := h.upsertUikLog(ctx, keyID, req, data, LogStatusExecError, "expression execution failed: "+err.Error())
if logErr != nil {
fmt.Println("failed to log UIK request:", logErr)
}
return
}

var insertedAny bool
for _, act := range actions {
inserted, err := h.handleAction(ctx, act)
if errutil.HTTPError(ctx, w, err) {
if err != nil {
logErr := h.upsertUikLog(ctx, keyID, req, data, LogStatusSendError, "action failed: "+err.Error())
if logErr != nil {
fmt.Println("failed to log UIK request:", logErr)
}
errutil.HTTPError(ctx, w, err)
return
}
insertedAny = insertedAny || inserted
Expand All @@ -172,5 +248,9 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
event.Send(ctx, h.evt, EventNewSignals{ServiceID: permission.ServiceNullUUID(ctx).UUID})
}

err = h.upsertUikLog(ctx, keyID, req, data, LogStatusSuccess, "")
if err != nil {
fmt.Println("failed to log UIK request:", err)
}
w.WriteHeader(http.StatusNoContent)
}
24 changes: 24 additions & 0 deletions migrate/migrations/20250722142521-uik-logs.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
-- +migrate Up

CREATE TABLE uik_logs (
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
integration_key_id uuid NOT NULL REFERENCES integration_keys(id) ON DELETE CASCADE,
Copy link
Member

Choose a reason for hiding this comment

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

nit/preference: we can just add UNIQUE to the column definition at table creation time instead of creating an explicit index below

integration_key_id uuid NOT NULL UNIQUE REFERENCES integration_keys(id) ON DELETE CASCADE,

received_at timestamptz NOT NULL DEFAULT now(),

status text NOT NULL CHECK (status IN (
'success',
'parse_error',
'exec_error',
'send_error'
)),

raw_body bytea,
content_type text,
user_agent text,
error_message text
);

CREATE UNIQUE INDEX uik_logs_unique_key_id ON uik_logs (integration_key_id);

-- +migrate Down
DROP TABLE IF EXISTS uik_logs;
24 changes: 21 additions & 3 deletions migrate/schema.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
-- This file is auto-generated by "make db-schema"; DO NOT EDIT
-- DATA=50f009ab810d42724027d31ec49f2584a1c8ecd647194c2455de853975d40bd0 -
-- DISK=2f8c154346b509ddac6e6d36a7a4f1bc86c047dcd1f98eb3ee5846bdf4bcf687 -
-- PSQL=2f8c154346b509ddac6e6d36a7a4f1bc86c047dcd1f98eb3ee5846bdf4bcf687 -
-- DATA=79a04167bec00db3465859780ff258857275b706408e83226c1bf579a33b5479 -
-- DISK=1072aa5cb3367eb1bc0e91e1bed2d100cfd6c4cd1f974cf062fe61e73abc967f -
-- PSQL=1072aa5cb3367eb1bc0e91e1bed2d100cfd6c4cd1f974cf062fe61e73abc967f -
--
-- pgdump-lite database dump
--
Expand Down Expand Up @@ -2523,6 +2523,24 @@ CREATE UNIQUE INDEX uik_config_primary_token_key ON public.uik_config USING btre
CREATE UNIQUE INDEX uik_config_secondary_token_key ON public.uik_config USING btree (secondary_token);


CREATE TABLE uik_logs (
content_type text,
error_message text,
id uuid DEFAULT gen_random_uuid() NOT NULL,
integration_key_id uuid NOT NULL,
raw_body bytea,
received_at timestamp with time zone DEFAULT now() NOT NULL,
status text NOT NULL,
user_agent text,
CONSTRAINT uik_logs_integration_key_id_fkey FOREIGN KEY (integration_key_id) REFERENCES integration_keys(id) ON DELETE CASCADE,
CONSTRAINT uik_logs_pkey PRIMARY KEY (id),
CONSTRAINT uik_logs_status_check CHECK (status = ANY (ARRAY['success'::text, 'parse_error'::text, 'exec_error'::text, 'send_error'::text]))
);

CREATE UNIQUE INDEX uik_logs_pkey ON public.uik_logs USING btree (id);
CREATE UNIQUE INDEX uik_logs_unique_key_id ON public.uik_logs USING btree (integration_key_id);


CREATE TABLE user_calendar_subscriptions (
config jsonb NOT NULL,
created_at timestamp with time zone DEFAULT now() NOT NULL,
Expand Down
2 changes: 1 addition & 1 deletion web/src/expflag.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// Code generated by expflag/cmd/tsgen DO NOT EDIT.

type ExpFlag = 'example' | 'univ-keys'
type ExpFlag = 'example' | 'uik-logs' | 'univ-keys'
Loading