Skip to content

fix(core): parse timestamps with any number of fractional-second digits - #9186

Open
breken-ai wants to merge 1 commit into
apache:mainfrom
breken-ai:fix/iso8601-variable-fraction-digits
Open

breken-ai wants to merge 1 commit into
apache:mainfrom
breken-ai:fix/iso8601-variable-fraction-digits

Conversation

@breken-ai

Copy link
Copy Markdown

Pre Checklist

  • I have read through the Contributing Documentation.
  • I have added relevant tests.
  • I have added relevant documentation. (No user-facing docs change; the new format entry has a comment.)
  • I will add labels to the PR, such as pr-type/bug-fix, pr-type/feature-development, etc. (I can't set labels from a fork. pr-type/bug-fix fits.)

Summary

ConvertStringToTime and ConvertStringToTimeInLoc (backend/core/models/common/iso8601time.go) match timestamps that end in a +hh:mm offset against fixed-width layouts: .000 (3 digits) and .000000 (6 digits, added in #8828). Any other number of fractional digits fails to parse:

input source before after
2026-09-29T10:00:00.1234567+08:00 .NET round-trip format, e.g. PowerShell Get-Date -Format o cannot parse "7+08:00" as "-07:00" parsed
2025-09-15T17:53:36.337932123+09:00 Go time.RFC3339Nano, Java OffsetDateTime with nanos parse error parsed
2025-09-15T10:53:36.3379+02:00 RFC3339Nano with trailing zeros trimmed parse error parsed

This matters because the same function backs the mapstructure decode hook (helpers/pluginhelper/api/mapstructure.go). So when a CI script posts a deployment to the webhook plugin with startedDate/finishedDate in one of these formats, DecodeMapStruct fails and postDeployments returns 400. That deployment is never stored, so deployment frequency, change lead time and failed deployment recovery time are all computed without it. The same thing happens to webhook incidents and issues. An Iso8601Time field in a plugin extractor fails its subtask the same way, which is what #8708 hit with 6 digits.

The fix replaces the two fixed-width entries with one entry, \.[\d]+[+-][\d]{2}:[\d]{2}$, using the layout 2006-01-02T15:04:05.999999999-07:00. That layout accepts a fraction of any length. The 3- and 6-digit cases parse exactly as before.

Does this close any open issues?

No open issue. This is a follow-up to #8708 / #8828, which fixed only the 6-digit case.

Screenshots

N/A. Test evidence:

  • New cases in TestConvertStringToTime (7, 9, 4 and 1 fractional digits) and TestConvertStringToTimeInLoc (7 digits).
  • On main (b0a75899): go test ./core/models/common/ -run TestConvertStringToTime -v fails the 7-, 9- and 4-digit cases and the 7-digit InLoc case (4 failures). The existing 3/6-digit cases and the 1-digit case pass.
  • With the fix, all 13 subtests pass and go test ./core/models/common/ is green. gofmt -l and go vet are clean.
  • I also checked the webhook path by calling api.DecodeMapStruct on a {startedDate, finishedDate} payload with 7- and 9-digit timestamps. It returns an error on main and decodes both values with the fix.

Other Information

I used an AI coding assistant (Claude Code) to write this change, following the ASF generative tooling guidance; the commit carries a Generated-by: trailer.

ConvertStringToTime and ConvertStringToTimeInLoc matched timestamps with a
`+hh:mm` offset against fixed-width layouts (".000" and ".000000"). Any
other precision failed: .NET's round-trip format (7 digits, e.g. PowerShell
`Get-Date -Format o`), Go's RFC3339Nano (up to 9 digits, trailing zeros
trimmed) and trimmed 4/5-digit values all returned a parse error.

This parser backs the mapstructure decode hook, so a webhook deployment or
incident posted with such a timestamp was rejected with 400 and never
counted, and an Iso8601Time field in an extractor failed the subtask.

Replace the two fixed-width entries with one entry that accepts any
fraction length through the ".999999999" layout.

Generated-by: Claude Code (Claude Opus 5.5)
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.

1 participant