feat(vercel): Add drain env variables to Vercel integration#103986
feat(vercel): Add drain env variables to Vercel integration#103986AbhiPrasad merged 2 commits intomasterfrom
Conversation
github-actions
bot
added
the
Scope: Backend
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #103986 +/- ##
===========================================
+ Coverage 76.12% 80.61% +4.49%
===========================================
Files 9311 9313 +2
Lines 397260 397439 +179
Branches 25356 25356
===========================================
+ Hits 302403 320404 +18001
+ Misses 94416 76594 -17822
Partials 441 441 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| .with_project(sentry_project) | ||
| .with_project_key(project_key) | ||
| .with_auth_token(sentry_auth_token) | ||
| .with_framework(vercel_project.get("framework")) |
There was a problem hiding this comment.
Bug: Framework validation fails when Vercel project lacks framework
The builder calls with_framework(vercel_project.get("framework")) which passes None when a Vercel project lacks a framework field. However, the build() method raises ValueError("framework is required") when framework is None. This breaks integration setup for Vercel projects without a detected framework, which is a valid scenario. The old implementation expected a string framework parameter, but the new code doesn't handle the missing framework case.
Additional Locations (1)
vgrozdanic
left a comment
vgrozdanic
left a comment
There was a problem hiding this comment.
Looks good to me, i just wouldn't raise the error if the framework is None
| if self._framework is None: | ||
| raise ValueError("framework is required") |
There was a problem hiding this comment.
Is the framework really required here?
From what i could tell it is only important to know if it is Next.js or not
Now that https://github.com/getsentry/getsentry/pull/18932 and #103986 has merged, we can remove the `get_env_var_map` as you should now just use the `VercelEnvVarMapBuilder` class instead.
2 participants
ref https://linear.app/getsentry/issue/LOGS-516/add-vercel-drain-urls-as-environmental-variables-to-vercel-integration
As part of our work to add on-click install drains for Vercel, I'm updating the Vercel integration to add three new environmental variables.
SENTRY_VERCEL_LOG_DRAIN_URL: The Vercel log drain URLSENTRY_OTLP_TRACES_URL: The OTLP traces URL that is used for vercel trace drainsSENTRY_PUBLIC_KEY: The public key that is used to authenticate with sentryTo get an idea of how these are used, check out the vercel drain docs: https://docs.sentry.io/product/drains/integration/vercel/
Previously we were populating these environmental variables with the
get_env_var_maphelper function. This is also used in thegetsentrycodebase. As this was difficult to manage, I replaced it with aVercelEnvVarMapBuilderbuilder class, that offers us more extensibility in case we need to add more environmental variables in the future. After we've updated theget_env_var_mapusage in thegetsentrycodebase, we can remove it here as well.To make the new environmental variables work with rpcs, I've had to serialize two new variables into
RpcProjectKey. These arepublic_key, used to populateSENTRY_PUBLIC_KEYandintegration_endpoint, used to populate bothSENTRY_VERCEL_LOG_DRAIN_URLandSENTRY_OTLP_TRACES_URL.I'm still not sure about the naming for
SENTRY_PUBLIC_KEY, so open to any feedback there for that.