Skip to content
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

fix nestjs sdk requiring endpoint to return #8276

Merged
Merged
5 changes: 5 additions & 0 deletions .changeset/chatty-adults-hammer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@highlight-run/node': minor
---

refactor highlight-run-with-headers to highlight-ctx
5 changes: 5 additions & 0 deletions .changeset/curly-lobsters-bathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@highlight-run/node': patch
---

update node sdk to export in larger batches
6 changes: 6 additions & 0 deletions .changeset/fuzzy-ties-repair.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@highlight-run/nest': minor
'@highlight-run/node': minor
---

fix nestjs trace context propagation and update node sdk span names
5 changes: 5 additions & 0 deletions .changeset/stale-poems-think.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@highlight-run/nest': patch
---

fix nest sdk breaking endpoints that do not return
4 changes: 2 additions & 2 deletions backend/clickhouse/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (client *Client) WriteErrorGroups(ctx context.Context, groups []*model.Erro
NewStruct(new(ClickhouseErrorGroup)).
InsertInto(ErrorGroupsTable, chGroups...).
BuildWithFlavor(sqlbuilder.ClickHouse)
sql, args = replaceTimestampInserts(sql, args, 10, map[int]bool{1: true, 2: true}, MicroSeconds)
sql, args = replaceTimestampInserts(sql, args, map[int]bool{1: true, 2: true}, MicroSeconds)
return client.conn.Exec(chCtx, sql, args...)
}

Expand Down Expand Up @@ -157,7 +157,7 @@ func (client *Client) WriteErrorObjects(ctx context.Context, objects []*model.Er
NewStruct(new(ClickhouseErrorObject)).
InsertInto(ErrorObjectsTable, chObjects...).
BuildWithFlavor(sqlbuilder.ClickHouse)
sql, args = replaceTimestampInserts(sql, args, 14, map[int]bool{1: true}, MicroSeconds)
sql, args = replaceTimestampInserts(sql, args, map[int]bool{1: true}, MicroSeconds)
return client.conn.Exec(chCtx, sql, args...)
}

Expand Down
12 changes: 8 additions & 4 deletions backend/clickhouse/insert.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"regexp"
"slices"
"strings"
)

type TimeUnit uint8
Expand All @@ -15,14 +16,17 @@ const (
NanoSeconds TimeUnit = 9
)

var pattern = regexp.MustCompile(`\?`)
var keysPattern = regexp.MustCompile(`^INSERT INTO \w+ \(([^)]+)\) VALUES`)
var argPattern = regexp.MustCompile(`\?`)

// replaceTimestampInserts updates direct timestamp inserts to accept int64 unix values
func replaceTimestampInserts(sql string, args []interface{}, numColumns int, columnsToReplace map[int]bool, scale TimeUnit) (string, []interface{}) {
func replaceTimestampInserts(sql string, args []interface{}, columnsToReplace map[int]bool, scale TimeUnit) (string, []interface{}) {
keysMatch := keysPattern.FindStringSubmatch(sql)
keys := strings.Split(keysMatch[1], ",")
var replaced, found int
sql = pattern.ReplaceAllStringFunc(sql, func(s string) string {
sql = argPattern.ReplaceAllStringFunc(sql, func(s string) string {
defer func() { found++ }()
idx := found % numColumns
idx := found % len(keys)
if !columnsToReplace[idx] {
return "?"
}
Expand Down
2 changes: 1 addition & 1 deletion backend/clickhouse/insert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ func Benchmark_replaceTimestampInserts(b *testing.B) {
for i := 0; i < b.N; i++ {
replaceTimestampInserts("INSERT INTO error_groups (ProjectID, CreatedAt, UpdatedAt, ID, Event, Status, Type, ErrorTagID, ErrorTagTitle, ErrorTagDescription) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", []interface{}{
1, ts, ts + 1, 2, "event", "status", "type", 3, "title", "description",
}, 10, map[int]bool{0: true, 1: true, 2: true, 7: true, 9: true}, NanoSeconds)
}, map[int]bool{0: true, 1: true, 2: true, 7: true, 9: true}, NanoSeconds)
}
assert.Less(b, b.Elapsed()/time.Duration(b.N), time.Millisecond)
}
4 changes: 2 additions & 2 deletions backend/clickhouse/sessions.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func (client *Client) WriteSessions(ctx context.Context, sessions []*model.Sessi
NewStruct(new(ClickhouseSession)).
InsertInto(SessionsTable, chSessions...).
BuildWithFlavor(sqlbuilder.ClickHouse)
sessionsSql, sessionsArgs = replaceTimestampInserts(sessionsSql, sessionsArgs, 34, map[int]bool{7: true, 8: true}, MicroSeconds)
sessionsSql, sessionsArgs = replaceTimestampInserts(sessionsSql, sessionsArgs, map[int]bool{7: true, 8: true}, MicroSeconds)
return client.conn.Exec(chCtx, sessionsSql, sessionsArgs...)
})
}
Expand All @@ -228,7 +228,7 @@ func (client *Client) WriteSessions(ctx context.Context, sessions []*model.Sessi
NewStruct(new(ClickhouseField)).
InsertInto(FieldsTable, chFields...).
BuildWithFlavor(sqlbuilder.ClickHouse)
fieldsSql, fieldsArgs = replaceTimestampInserts(fieldsSql, fieldsArgs, 6, map[int]bool{3: true}, MicroSeconds)
fieldsSql, fieldsArgs = replaceTimestampInserts(fieldsSql, fieldsArgs, map[int]bool{3: true}, MicroSeconds)
return client.conn.Exec(chCtx, fieldsSql, fieldsArgs...)
})
}
Expand Down
2 changes: 1 addition & 1 deletion docs-content/sdk/nodejs.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ slug: nodejs
<section className="section">
<div className="left">
<h3>H.runWithHeaders</h3>
<p>H.runWithHeaders() wraps its callback with a span named highlight-run-with-headers, and spreads Highlight's secureSessionId and requestId across all spans with matching a traceId.</p>
<p>H.runWithHeaders() wraps its callback with a span named highlight-ctx, and spreads Highlight's secureSessionId and requestId across all spans with matching a traceId.</p>
<h6>Method Parameters</h6>
<aside className="parameter">
<h5>headers<code>IncomingHttpHeaders</code> <code>required</code></h5>
Expand Down
19 changes: 14 additions & 5 deletions e2e/express-ts/src/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import express from 'express'
import { H, Handlers } from '@highlight-run/node'
import { CONSTANTS } from './constants'
import { config } from './instrumentation'
import { context } from '@opentelemetry/api'

H.init(config)

Expand All @@ -11,17 +12,25 @@ const port = 3003
// This should be before any controllers (route definitions)
app.use(Handlers.middleware(config))
app.get('/', async (req, res) => {
await H.runWithHeaders(req.headers, () => {
await H.runWithHeaders(req.headers, async () => {
const err = new Error('this is a test error', {
cause: { route: '/', foo: ['bar'] },
})
console.info('Sending error to highlight')
H.consumeError(err)
H.consumeError(
new Error('this is another test error', {
cause: 'bad code',
}),
const { span, ctx } = H.startWithHeaders(
'second-error',
req.headers,
{},
)
context.with(ctx, () => {
H.consumeError(
new Error('this is another test error', {
cause: 'bad code',
}),
)
})
span.end()

res.send('Hello World!')
})
Expand Down
9 changes: 7 additions & 2 deletions e2e/nestjs/src/app.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@ export class AppController {

@Get()
async getHello(): Promise<string[]> {
return await this.appService.findAll();
return await this.appService.findAll({});
}

@Get('/empty')
async getEmpty() {
return await this.appService.findAll({ empty: true });
}

@Get('/error')
async getError(): Promise<string[]> {
return await this.appService.findAll(true);
return await this.appService.findAll({ error: true });
}
}
28 changes: 23 additions & 5 deletions e2e/nestjs/src/app.service.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
import { Injectable, Logger } from '@nestjs/common';
import { firstValueFrom } from 'rxjs';
import { HttpService } from '@nestjs/axios';
import { H } from '@highlight-run/nest';

@Injectable()
export class AppService {
private readonly logger = new Logger(AppService.name);
constructor(private readonly httpService: HttpService) {}

async findAll(throwError: boolean = false): Promise<string[]> {
async findAll({
error,
empty,
}: {
error?: true;
empty?: true;
}): Promise<string[]> {
const span = await H.startActiveSpan('making request now', {});
await firstValueFrom(
this.httpService.post<any[]>(
'https://pub.highlight.io/v1/logs/json',
Expand All @@ -24,13 +32,23 @@ export class AppService {
},
),
);
span.end();

this.logger.log('hello, world!');
this.logger.warn('whoa there! ', Math.random());
if (throwError) {
for (let i = 0; i < 10; i++) {
const s = await H.startActiveSpan(`another request ${i}`, {
attributes: { i },
});
console.log('hello, world!');
this.logger.log('hello, world!');
this.logger.warn('whoa there! ', Math.random());
s.end();
}
if (error) {
// error will be caught by the HighlightErrorFilter
throw new Error(`a random error occurred!`);
}
return [`Hello World!`];
if (!empty) {
return [`Hello World!`];
}
}
}
2 changes: 1 addition & 1 deletion e2e/nestjs/src/main.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { NestFactory } from '@nestjs/core';
import { AppModule } from './app.module';
import { HighlightInterceptor, H } from '@highlight-run/nest';

const env = {
projectID: '2',
Expand All @@ -11,6 +10,7 @@ const env = {
};

async function bootstrap() {
const { HighlightInterceptor, H } = await import('@highlight-run/nest');
H.init(env);
const app = await NestFactory.create(AppModule);
app.useGlobalInterceptors(new HighlightInterceptor(env));
Expand Down
2 changes: 1 addition & 1 deletion sdk/highlight-apollo/src/apollo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('ApolloServerHighlightPlugin', () => {
const { details } = await getResourceSpans(port)

const highlightRunWithHeadersSpan = details.find(({ spanNames }) =>
spanNames.includes('highlight-run-with-headers'),
spanNames.includes('highlight-ctx'),
)
const sessionId =
highlightRunWithHeadersSpan?.attributes['highlight.session_id']
Expand Down
2 changes: 1 addition & 1 deletion sdk/highlight-apollo/src/apolloV3.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('ApolloServerV3HighlightPlugin', () => {
const { details } = await getResourceSpans(port)

const highlightRunWithHeadersSpan = details.find(({ spanNames }) =>
spanNames.includes('highlight-run-with-headers'),
spanNames.includes('highlight-ctx'),
)
const sessionId =
highlightRunWithHeadersSpan?.attributes['highlight.session_id']
Expand Down
59 changes: 31 additions & 28 deletions sdk/highlight-nest/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import {
Injectable,
NestInterceptor,
} from '@nestjs/common'
import { lastValueFrom, Observable, throwError } from 'rxjs'
import { finalize, Observable, throwError } from 'rxjs'
import { catchError } from 'rxjs/operators'
import api from '@opentelemetry/api'

@Injectable()
export class HighlightLogger
Expand Down Expand Up @@ -93,36 +94,38 @@ export class HighlightInterceptor
await NodeH.flush()
}

intercept(
context: ExecutionContext,
next: CallHandler,
): Promise<Observable<any>> {
intercept(context: ExecutionContext, next: CallHandler): Observable<any> {
const ctx = context.switchToHttp()
const request = ctx.getRequest()
const highlightCtx = NodeH.parseHeaders(request.headers)
return NodeH.runWithHeaders(request.headers, async () => {
const span = await NodeH.startActiveSpan(
`${request.method} ${request.url}`,
{
attributes: {
'http.method': request.method,
'http.url': request.url,
},

const { span: requestSpan, ctx: spanCtx } = NodeH.startWithHeaders(
`${request.method} ${request.url}`,
request.headers,
{
attributes: {
'http.method': request.method,
'http.url': request.url,
},
)
try {
return await lastValueFrom(next.handle())
} catch (err: any) {
NodeH.consumeError(
err,
highlightCtx?.secureSessionId,
highlightCtx?.requestId,
)
throw err
} finally {
span.end()
}
})
},
)
const fn = api.context.bind(spanCtx, () =>
next.handle().pipe(
catchError((err) => {
NodeH.consumeError(
err,
undefined,
undefined,
{},
{ span: requestSpan },
)
return throwError(() => err)
}),
finalize(() => {
requestSpan.end()
}),
),
)
return fn()
}
}

Expand Down
Loading
Loading