Skip to content

[pkg/ottl] Adapt time parsing error messages to include the ctime directives #38425

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

Conversation

bacherfl
Copy link
Contributor

@bacherfl bacherfl commented Mar 6, 2025

Description

This PR adapts the Time function to check for time.ParseError errors. If such an error is received by the function, it wraps it into a custom error type in order to include the provided ctime layout string instead of the go-native layout string.

Link to tracking issue

Fixes #35176

Testing

Adapted the unit tests of the Time() function

bacherfl added 2 commits March 6, 2025 08:21
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
…rmat

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
bacherfl added 3 commits March 7, 2025 07:53
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
@bacherfl bacherfl marked this pull request as ready for review March 10, 2025 09:18
@bacherfl bacherfl requested a review from a team as a code owner March 10, 2025 09:18
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot removed the Stale label Apr 30, 2025
@atoulme atoulme marked this pull request as draft May 8, 2025 00:51
@atoulme
Copy link
Contributor

atoulme commented May 8, 2025

Moving to draft while you address the conflict and review, please mark ready for review once addressed.

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label May 22, 2025
@github-actions github-actions bot removed the Stale label May 26, 2025
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
bacherfl added 6 commits May 26, 2025 08:03
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
…ssages' into feat/35176/time-parse-error-messages
@bacherfl bacherfl marked this pull request as ready for review May 27, 2025 05:22
Copy link
Contributor

@edmocosta edmocosta left a comment

Choose a reason for hiding this comment

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

LGTM, just left some final suggestions.

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Copy link
Contributor

@edmocosta edmocosta left a comment

Choose a reason for hiding this comment

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

Thank you for working on that, LGTM!

@atoulme atoulme merged commit 831984b into open-telemetry:main May 29, 2025
177 checks passed
@github-actions github-actions bot added this to the next release milestone May 29, 2025
dd-jasminesun pushed a commit to DataDog/opentelemetry-collector-contrib that referenced this pull request Jun 23, 2025
…ectives (open-telemetry#38425)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This PR adapts the `Time` function to check for `time.ParseError`
errors. If such an error is received by the function, it wraps it into a
custom error type in order to include the provided ctime layout string
instead of the go-native layout string.

<!-- Issue number (e.g. #1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#35176 

<!--Describe what testing was performed and which tests were added.-->
#### Testing

Adapted the unit tests of the `Time()` function

---------

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pkg/ottl] Improve Time converter error messages
6 participants