Vercel Integration: Log Pipeline & Metrics corrections & improvements - #3123
Vercel Integration: Log Pipeline & Metrics corrections & improvements#3123swanson8r wants to merge 110 commits into
Conversation
Nested pipeline with 3 processors
@Environment attribute can be used to populate the env tag
add metric groupings by env and version
Added new Vercel-related fields for environment, deployment, invocation, project IDs, names, and request duration. Updated status code remapper queries for better log categorization.
Updated paths for Vercel attributes to include 'vercel.' prefix and added attribute remappers for various Vercel identifiers. Apply new prefixed attributes to Build Count Log Metric.
Format similar to [existing configs](https://github.com/search?q=repo%3ADataDog%2Fintegrations-extras+path%3A**%2Fassets%2Flogs%2F*.yaml+-path%3A**%2F*tests.yaml+facetType&type=code), sort alphabetically
process of elimination to troubleshoot repo assets validation
Removed multiple metrics related to Vercel functions from the configuration.
Restoring pipeline config to the basics until the log asset validation stops failing with internal errors
Updated log structure and added new fields for URL details.
Begin restoring log config additions now that validation is passing
Removed various Vercel deployment-related facets from the YAML configuration.
leave out the lambda measure
swallowed validation error
Test a single attribute remapper and facet different from the one that passed last time
Added invocationId and updated deploymentId in logs.
| metrics: | ||
| - name: vercel.functions.invocations |
There was a problem hiding this comment.
Was getting rid of this section intentional?
There was a problem hiding this comment.
Thanks for taking a look! As you can infer from the pile of commits and failed asset-validation tests, I have been removing and re-adding portions of the log configurations to guess-and-check my way to success, in the absence of clear documentation about failure details hidden in a private repo ( https://github.com/apps/datadog-assets?pr=3123 ).
The specific commit where these were removed is: - f88e772
There was a problem hiding this comment.
Log Metrics have been restored, along with updates to match the new remapped vercel. attributes, and some additional groupBys as mentioned in the PR description.
- The most notable addition is
deployment_id, which functions as aversiontag.
| query: '@proxy.statusCode:[400 TO 499] OR (@type:stderr AND @source:build) OR message:*warn*' | ||
| name: Warning | ||
| - filter: | ||
| query: '@proxy.statusCode:[500 TO 599]' | ||
| query: '@proxy.statusCode:[500 TO 599] OR (@type:stderr AND NOT @source:build AND NOT message:*warn*)' |
There was a problem hiding this comment.
What led you to differentiate on build logs and exclude them from the Error category here?
There was a problem hiding this comment.
Great question, one which can be addressed by adding another sample log to be tested.
Vercel Build logs do not contain an HTTP Status Code, so the next best option is to rely on the @type attribute.
On the downside, these logs are currently emitted one line at a time instead of in json format, which causes headaches when there are stack traces, and requires querying as Transactions in order to assemble as a single end-to-end build by @buildId
| query: '@type:stdout AND @source:build' | ||
| name: Info | ||
| - filter: | ||
| query: '@proxy.statusCode:[200 TO 299]' | ||
| name: OK | ||
| - filter: | ||
| query: '@proxy.statusCode:[300 TO 399]' | ||
| name: Notice | ||
| - filter: | ||
| query: '@proxy.statusCode:[400 TO 499]' | ||
| query: '@proxy.statusCode:[400 TO 499] OR (@type:stderr AND @source:build) OR message:*warn*' |
There was a problem hiding this comment.
Do you know of a more definitive way to parse the warning messages than *warn*? Other logs pipelines (example) have access to a level field, does that exist here?
There was a problem hiding this comment.
I agree that a bareword search for Warn in a Vercel log message body is weak. Perhaps we can arrive at a better solution?
The reason I added this was for warnings where an HTTP Status Code does not exist at all, but there is non-OK log output (this problem is not limited to build logs).
- These logs can be queried by searching for:
source:vercel -_exists_:@proxy.statusCode message:*warn* -@source:build -_exists_:@http.status_code
- A quick assessment can be made by grouping into Fields and Visualize as a Table by
status,@type, and@level- Changing to Group into Patterns will help to find the most common examples that require this special treatment
Aggregate results in the past 15 days of log retention:
| Status | Type | @Level | count |
|---|---|---|---|
| 2,353.32k | |||
| warn | 2,342.54k | ||
| stderr | 2,340.64k | ||
| warning | 1,643.51k | ||
| error | 358.97k | ||
| warn | 338.16k | ||
| stdout | 1.91k | ||
| info | 1.91k | ||
| info | 10.77k | ||
| stdout | 10.77k | ||
| info | 10.77k | ||
| error | 5 | ||
| stderr | 5 | ||
| error | 5 |
|
Hi, thanks for reaching out, I left a few comments on the PR. I'll want to double check that everything in this works with our existing UI (once the metrics get added back in) before we merge, but this change looks reasonable to me.
If you could add these it would be helpful for validating this. |
Will need to revisit remapper to ensure span_id is not eliminated in logs
Per https://docs.datadoghq.com/getting_started/tagging/#define-tags , tag values are converted to lowercase. > [!NOTE] > This will cause some mismatches between the `version` tag and the actual Vercel deploymentId, which is mixed case.
Added new attribute remappers for deployment environment and updated existing mappings.
Removed projectId and projectName from the main section and added them under the vercel section.
Removed redundant duration field from lambda section.
Removed the Vercel Request Duration metric and added multiple new metrics for function invocations, errors, duration, execution, max memory used, memory size, requests, and build count.
Added duration field to the lambda execution details.
Removed lambda duration from Vercel logs.
Removed mapping for `lambda.duration` to `vercel.lambda.duration` and updated aggregation path.
swanson8r
left a comment
swanson8r
left a comment
There was a problem hiding this comment.
Each category-processor status has been commented on with a markdown table of query results from the past 15d of log retention in source:@vercel
I can share obfuscated Patterns of any that are questionable, and have already identified one status filter query to update.
As only 4 attributes or tags can be grouped by at a time, here is an alternate view by status with @http.status_category replacing @source
Info
@http.status_category |
Status | @level |
Type | count |
|---|---|---|---|---|
| 5,403.5k | ||||
| Info | 5,365.5k | |||
| info | 5,365.5k | |||
| info | 5,365.5k | |||
| stdout | 5,365.5k | |||
| N/A | 24.1k | |||
| info | 24.1k | |||
| info | 24.1k | |||
| stdout | 24.1k | |||
| Warning | 13.9k | |||
| warn | 13.9k | |||
| info | 13.9k | |||
| stdout | 13.9k |
OK
(updated with proposed changes after noticing non-OK results)
@http.status_category |
Status | @level |
Type | count |
|---|---|---|---|---|
| 543M | ||||
| OK | 543M | |||
| ok | 543M | |||
| info | 543M | |||
| stdout | 543M | |||
| N/A | 8.98k | |||
| middleware-invocation | 689 |
Warning
@http.status_category |
Status | @level |
Type | count |
|---|---|---|---|---|
| 3,513.5k | ||||
| Warning | 2,951.1k | |||
| warn | 2,951.1k | |||
| warning | 2,180.9k | |||
| stderr | 1,315.3k | |||
| stdout | 865.2k | |||
| N/A | 429 | |||
| info | 482.3k | |||
| stderr | 482.3k | |||
| warn | 287.9k | |||
| stderr | 287.9k | |||
| error | 1 | |||
| stderr | 1 | |||
| N/A | 519.7k | |||
| warn | 519.7kv | |||
| warning | 469.1k | |||
| stderr | 469.1k | |||
| warn | 50.6k | |||
| stderr | 50.6k | |||
| Error | 42.7k | |||
| error | 42.7k | |||
| warning | 42.7k | |||
| stderr | 42.7k |
Error
@http.status_category |
Status | @level |
Type | count |
|---|---|---|---|---|
| 2,388k | ||||
| Error | 1,307k | |||
| error | 1,307k | |||
| error | 1,307k | |||
| stderr | 1,307k | |||
| N/A | 5 | |||
| Warning | 645k | |||
| warn | 645k | |||
| error | 645k | |||
| stderr | 645k | |||
| N/A | 436k | |||
| error | 436k | |||
| error | 436k | |||
| stderr | 436k |
Notice
(Unchanged; not added as a separate filter comment)
source:vercel status:notice
@http.status_category |
@level |
Type | @source |
count |
|---|---|---|---|---|
| 11.07M | ||||
| Notice | 11.07M | |||
| info | 11.07M | |||
| stdout | 11.07M | |||
| lambda | 11.07M | |||
| middleware-invocation | 74 | |||
| edge | 74 | |||
| N/A | 1 | |||
| lambda | 1 |
| - filter: | ||
| query: '@type:stdout AND (@source:build OR (@level:info AND @type:stdout AND NOT _exists_:@http.status_code AND NOT _exists_:@proxy.statusCode))' | ||
| name: Info |
There was a problem hiding this comment.
| Status | Type | @level |
@source |
count |
|---|---|---|---|---|
| 5,388.4k | ||||
| info | 5,374.5k | |||
| stdout | 5,374.5k | |||
| info | 5,374.5k | |||
| build | 5,345.4k | |||
| lambda | 29.1k | |||
| edge | 4 | |||
| warn | 13.8k | |||
| stdout | 13.8k | |||
| info | 13.8k | |||
| build | 11.9k | |||
| lambda | 1.9k |
| - filter: | ||
| query: '@proxy.statusCode:[400 TO 499]' | ||
| query: '@proxy.statusCode:[400 TO 499] OR (@type:stderr AND (@source:build OR (@level:warn* AND @type:stderr AND NOT _exists_:@http.status_code AND NOT _exists_:@proxy.statusCode)))' | ||
| name: Warning |
There was a problem hiding this comment.
| Status | Type | @level |
@source |
count |
|---|---|---|---|---|
| 3,511.7k | ||||
| warn | 3,469.1k | |||
| stderr | 2,605.0k | |||
| warning | 1,784.3k | |||
| lambda | 1,784.3k | |||
| info | 482.3k | |||
| build | 482.3k | |||
| warn | 338.4k | |||
| lambda | 338.4k | |||
| error | 1 | |||
| lambda | 1 | |||
| stdout | 863.6k | |||
| warning | 863.6k | |||
| lambda | 863.6k | |||
| N/A | 429 | |||
| warning | 429 | |||
| firewall | 379 | |||
| lambda | 50 | |||
| error | 42.7k | |||
| stderr | 42.7k | |||
| warning | 42.7k | |||
| lambda | 42.7k |
| query: '@proxy.statusCode:[500 TO 599] OR (@type:stderr AND NOT @source:build AND @level:error AND NOT _exists_:@http.status_code AND NOT _exists_:@proxy.statusCode)' | ||
| name: Error |
There was a problem hiding this comment.
| Status | Type | @level |
@source |
count |
|---|---|---|---|---|
| 2,387k | ||||
| error | 1,742k | |||
| stderr | 1,742k | |||
| error | 1,742k | |||
| lambda | 1,742k | |||
| N/A | 5 | |||
| error | 5 | |||
| lambda | 5 | |||
| warn | 646k | |||
| stderr | 646k | |||
| error | 646k | |||
| lambda | 646k |
Per [datadog-assets / validate-logs ](https://github.com/DataDog/integrations-extras/pull/3123/checks?check_run_id=98493321185) run after committing log metrics changes
| sourceType: attribute | ||
| target: vercel.projectName | ||
| targetType: attribute | ||
| preserveSource: false |
There was a problem hiding this comment.
Our Vercel page on the Datadog UI depends on some of these metrics, I know we want the vercel. prefix for the custom facets but we should set preserveSource to true for anything that already exists
There was a problem hiding this comment.
I am happy to set preserveSource: true as an interim step.
❓ Could the Vercel Serverless Page be updated to also query from log attributes prefixed with @vercel.*?
Note
My understanding of the recommended practices according to Datadog's Create a Log Pipeline document are that integration metrics should be prefixed with the log source name.
Here are the relevant snippets:
For non-reserved standard attributes, use the general Remapper.
Unless updating an existing integration log pipeline, disablePreserve source attributeto prevent duplicate values.
Custom facets must include a prefix matching your logs source.
For example, a facet on project_name should use thepath @<logs_source>.project_name.
| name: Warning | ||
| - filter: | ||
| query: '@proxy.statusCode:[500 TO 599]' | ||
| query: '@proxy.statusCode:[500 TO 599] OR (@type:stderr AND NOT @source:build AND @level:error AND NOT _exists_:@http.status_code AND NOT _exists_:@proxy.statusCode)' |
There was a problem hiding this comment.
One case here that isn't categorized is if someone is logging everything through stdout, but has levels setup. I believe those would end up being falling through all of the queries here unless they also are attaching a status code to the log lines.
There was a problem hiding this comment.
Interesting scenario! Querying our own logs for source:vercel @type:stdout @level:error, we don't have any matches.
Checking next to see if we ever have the opposite fall through, source:vercel @type:stderr NOT @level:error grouped by @level, status returns a mishmash, which is why we chose to require both criteria
Here is one option to accommodate your scenario by removing @type entirely:
| query: '@proxy.statusCode:[500 TO 599] OR (@type:stderr AND NOT @source:build AND @level:error AND NOT _exists_:@http.status_code AND NOT _exists_:@proxy.statusCode)' | |
| query: '@proxy.statusCode:[500 TO 599] OR (@level:error AND NOT @source:build AND NOT _exists_:@http.status_code AND NOT _exists_:@proxy.statusCode)' |
This produces a cleaner result. I can review our logs follow up with a comment any currently marked as warn that should stay that way.
2 participants
What does this PR do?
Update
assets/logs/vercel.yamlversiontag of the log as the Vercel Deployment IDNote
Per https://docs.datadoghq.com/getting_started/tagging/#define-tags , tag values are converted to lowercase.
This will cause some mismatches between the
versiontag and the actual Vercel deploymentId, which is mixed case.envtag of the log as the Vercel Deployment Environmentvercel.prefixed attributes required for Custom FacetsMotivation
Experience using Vercel logs in a production environment has resulted in some frustrations properly identifying errors, and utilizing the generated metrics. Problems encountered include:
status:errorenvandversionimpeded query filter and groupingReview checklist
no-changeloglabel attachedAdditional Notes
I am happy to provide anonymized screenshots / JSON for any of the above stated motivating problems upon request.
Validation Workflow Errors
PR Validation Tests were failing, requiring trial and error to narrow down the impacting changes.
I was unfamiliar with the required syntax for updating and adding tests to
vercel/assets/logs/vercel_tests.yaml, but was able to follow the validation output errors once changes were limited enough to reveal them.ddev validate config vercel, received errorDid not find spec file <mypath>/integrations-extras/vercel/assets/configuration/spec.yaml for check vercel, this is not the same as the asset validation tests.assets/configuration/spec.yamlis absent in this integration, but does not appear to be required for log validation executed from pinned version of run-validations.yamlFileNotFoundError ... integrations-extras/requirements-agent-release.txtLicense validation is only available for repo \core`, skipping for repoextrasVersion validation is only available for repocore, skipping for repo \extras``At a minimum, tests should include:
@http.url_details.*and@http.referer_details.*attributes@source:buildand logs with no@proxy.statusCode, but other indications of an Error or Warning.vercel.projects.build_countactually groups by the assigned tags.