Skip to content
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

Fixed Timestamp not being set for otel log signals #44507

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

Chris18230
Copy link
Contributor

When using the otel logging, the log messages where displayed wrong in the Elastic Stack because the Timestamp was not set to the Log Signal. The Reason for this is that APM-Server is using the Timestamp and not the ObservedTimestamp for displaying the Message. For this reason they where display in the year 1970 in Elastic Stack.

Copy link

quarkus-bot bot commented Nov 14, 2024

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

  • title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

Copy link

quarkus-bot bot commented Nov 14, 2024

/cc @brunobat (opentelemetry), @radcortez (opentelemetry)

@Chris18230 Chris18230 changed the title fixed Timestamp not being set for otel log signals Fixed Timestamp not being set for otel log signals Nov 14, 2024
@geoand
Copy link
Contributor

geoand commented Nov 14, 2024

Thanks for the contribution!

Please rebase the PR onto main so it does not include merge commits.

@gsmet
Copy link
Member

gsmet commented Nov 14, 2024

git config --global pull.rebase true might be helpful

@gsmet
Copy link
Member

gsmet commented Nov 14, 2024

I just rebased it with the GitHub UI.

@gsmet gsmet requested a review from brunobat November 14, 2024 13:14

This comment has been minimized.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me, but let's let @brunobat decide

Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

Actually, it needs to be current time.

Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

Please check comments above.

Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

Thanks!

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Nov 15, 2024
@gsmet
Copy link
Member

gsmet commented Nov 15, 2024

The history is problematic, I will fix it.

This comment has been minimized.

Copy link

quarkus-bot bot commented Nov 15, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 3c19050.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@gsmet gsmet merged commit 814b8cd into quarkusio:main Nov 15, 2024
29 checks passed
@gsmet
Copy link
Member

gsmet commented Nov 15, 2024

Thanks for the fix! It will be included in the next 3.16.

@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Nov 15, 2024
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Nov 15, 2024
@gsmet gsmet modified the milestones: 3.18 - main, 3.16.4 Nov 19, 2024
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.

4 participants