Skip to content

feat(captcha): improve telemetry on captcha error #12836

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

Merged
merged 3 commits into from
Jun 24, 2025

Conversation

AMoreaux
Copy link
Contributor

No description provided.

@AMoreaux AMoreaux requested a review from charlesBochet June 24, 2025 14:18
@AMoreaux AMoreaux self-assigned this Jun 24, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Enhanced error telemetry across captcha validation system by standardizing error messages and adding detailed metrics tracking.

  • Standardized error messages in both TurnstileDriver and GoogleRecaptchaDriver from 'Captcha Error' to 'unknown-error' for better error tracking
  • Enhanced MetricsService in packages/twenty-server/src/engine/core-modules/metrics/metrics.service.ts to support attributes in telemetry counters
  • Added error attributes to metrics in CaptchaGuard for improved failure tracking and debugging
  • Aligned with OpenTelemetry standards for better integration with monitoring systems

4 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile

Comment on lines 34 to 36
...(!responseData.success && {
error: responseData['error-codes']?.[0] ?? 'Captcha Error',
error: responseData['error-codes']?.[0] ?? 'unknown-error',
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider adding a constant for 'unknown-error' to maintain consistency and avoid magic strings

shouldStoreInCache = true,
}: {
key: MetricsKeys;
eventIds: string[];
attributes?: Attributes;
Copy link
Member

Choose a reason for hiding this comment

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

how does this work? can a counter have attributes? how does this aggregates?

AMoreaux added 2 commits June 24, 2025 16:22
…error-telemetry

# Conflicts:
#	packages/twenty-server/src/engine/core-modules/metrics/metrics.service.ts
@AMoreaux AMoreaux enabled auto-merge (squash) June 24, 2025 14:31
Copy link
Contributor

github-actions bot commented Jun 24, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:6942

This environment will automatically shut down when the PR is closed or after 5 hours.

Copy link
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

LGTM!

@AMoreaux AMoreaux merged commit b7e72c3 into main Jun 24, 2025
42 checks passed
@AMoreaux AMoreaux deleted the feat/improve-captcha-error-telemetry branch June 24, 2025 15:43
abdulrahmancodes pushed a commit to abdulrahmancodes/twenty that referenced this pull request Jun 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants