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

Journal: Add message ids for emergency log messages #28873

Merged
merged 1 commit into from Sep 1, 2023

Conversation

1awesomeJ
Copy link
Contributor

There's a need to provide more details and possible fixes for all our emergency log messages.
So we're adding a UUID to each existing message, and making an entry for each of them in the catalog using this UUID.
We'll also be modifying the log_emergency() and log_emergency_errno() functions to require a MESSAGE_ID going forward.

The ultimate goal is to help users find meaningful help when they encounter such emergency messages.

@github-actions github-actions bot added the please-review PR is ready for (re-)review by a maintainer label Aug 17, 2023
@1awesomeJ
Copy link
Contributor Author

@bluca, @poettering
I'm starting with src/core/crash-handler.c
The file has 13 of our existing 23 log_emergency* calls.

Kindly pitch in or the error descriptions and the possible fixes in your spare minutes sirs.

Thank you.

@bluca
Copy link
Member

bluca commented Aug 18, 2023

Don't worry about the message text for now, focus on the implementation so that they can be used and displayed

@bluca bluca added journal and removed please-review PR is ready for (re-)review by a maintainer labels Aug 18, 2023
@1awesomeJ
Copy link
Contributor Author

Okay.
Good morning sir.

I did get 7 hours of sleep.
Just saying.

@1awesomeJ 1awesomeJ force-pushed the catalog branch 2 times, most recently from 2a8707c to 5b0873d Compare August 18, 2023 13:53
@1awesomeJ
Copy link
Contributor Author

@bluca,
Good afternoon sir.
I just finished working on the log_emergency*() calls.
There are 30 of them total, I worked on 29 in all.
The last one is made in a Coccinelle file (SmPL) It's somewhat different.

I assigned a UUID to each of the 29 messages, and I added an entry for them with these UUIDs in the catalog.

@1awesomeJ
Copy link
Contributor Author

Now I need to modify the log_emergency*() calls to take these UUIDs as MESSAGE_ID.

I found the functions are variadic, so they have room for the extra arg.

The issue I have is how the MESSAGE_ID will get used.

@1awesomeJ
Copy link
Contributor Author

I traced the calls like so:
log_emergency(r, "emergency : %m") -> log_full(LOG_EMERG, r, "emergency: %m") (if PID =1),
then log_full_errno_zerook(LOG_EMERG, 0, r, "emergency: %m") then log_internal() then log_internalv() then log_dispatch_internal() then write_to_journal() then log_do_header(), log_do_context(), and sendmsg()

@1awesomeJ
Copy link
Contributor Author

I haven't found where exactly the MESSAGE_ID should be used

Can you guide on that sir?

Thank you so so much.

@1awesomeJ 1awesomeJ marked this pull request as ready for review August 18, 2023 14:06
@github-actions github-actions bot added the please-review PR is ready for (re-)review by a maintainer label Aug 18, 2023
@bluca
Copy link
Member

bluca commented Aug 18, 2023

Take a look at log_struct()

@1awesomeJ
Copy link
Contributor Author

Take a look at log_struct()

Okay sir.

Please can I ask questions till 10pm tonight?

And maybe 2 questions tomorrow.

I'd like this to be ready for testing by Monday.

Thank you sir.

@bluca bluca added 🙋🏽‍♀️ outreachy 🙋 and removed please-review PR is ready for (re-)review by a maintainer labels Aug 18, 2023
@bluca bluca linked an issue Aug 18, 2023 that may be closed by this pull request
@1awesomeJ
Copy link
Contributor Author

@bluca,
Good evening sir.
Did you have an amazing day?

I've looked at the log_struct(). Like the logging functions which eventually lead to log_internal(), log_struct() also funnels down to log_struct_internal()

So, I'll spend time tomorrow studying these two functions in details: log_struct_internal() and log_dispatch_internal

I didn't work on it again after we last communicated.

I decided to watch some TV for the first time in about 12 months since I started learning programming.

Thank you for being a great mentor.
You guys have made the journey awesome for me. "Thank you" to everyone too.

@1awesomeJ
Copy link
Contributor Author

Before I go back to NFLX though,
Here's the current prototype of log_struct_internal():

int log_struct_internal(
                int level,
                int error,
                const char *file,
                int line,
                const char *func,
                const char *format, ...) _printf_(6,0) _sentinel_;

I'm thinking we should add a sd_id128 message_id field

@bluca
Copy link
Member

bluca commented Aug 18, 2023

that is not necessary, it is already possible to use it, check how MESSAGE_ID is used elsewhere in src/, it happens often

@1awesomeJ
Copy link
Contributor Author

I realized that too.
After seeing that there was a call to sd_journal_add_match() with the filter "MESSAGE_ID="

@bluca
Copy link
Member

bluca commented Aug 19, 2023

I will be away for a week, but @poettering will be able to help you out

@1awesomeJ
Copy link
Contributor Author

I will be away for a week, but @poettering will be able to help you out

Ohhh ..

  1. I hope it's a vacation.
    Like boat rides and all.

  2. I hope I'll get guidance from you still when you get back sir.

Thank you so much for all I've gotten these past months.
Have an amazing trip sir.
Thank you so much!!!

@1awesomeJ
Copy link
Contributor Author

1awesomeJ commented Aug 19, 2023

@poettering,
I'm considering changing the definitions of the log_emergency*() functions like so:

static inline int log_emergency(const char *id, const char *message) {
          return log_struct(LOG_EMERGENCY,
                                     LOG_MESSAGE(message),
                                     "MESSAGE_ID=" id);
}

However to accomodate the va_args, will this be better:

static inline int log_emergency(const char *id, const char *message, ...) {
          return log_struct(LOG_EMERGENCY,
                                     LOG_MESSAGE(message),
                                     "MESSAGE_ID=" id, ...);
}

what do you advise sir?

@1awesomeJ
Copy link
Contributor Author

If that is okay,
I will have to manually update the 30 existing calls to log_emergency() and log_emergency_errno(),
and perhaps some docs?

@1awesomeJ
Copy link
Contributor Author

@poettering,
Good morning sir.

@1awesomeJ
Copy link
Contributor Author

1awesomeJ commented Aug 30, 2023

We can the use then message id to fetch the entry from the catalog somehow,
Then we'll have a webpage for that entry, and its url will be passed to our print_qrcode() call.

src/basic/process-util.c Outdated Show resolved Hide resolved
src/core/crash-handler.c Outdated Show resolved Hide resolved
@1awesomeJ
Copy link
Contributor Author

I do appreciate your time on this sir.
Thank you!

src/core/crash-handler.c Outdated Show resolved Hide resolved
src/core/crash-handler.c Outdated Show resolved Hide resolved
catalog/systemd.catalog.in Outdated Show resolved Hide resolved
catalog/systemd.catalog.in Outdated Show resolved Hide resolved
catalog/systemd.catalog.in Outdated Show resolved Hide resolved
catalog/systemd.catalog.in Outdated Show resolved Hide resolved
@1awesomeJ
Copy link
Contributor Author

On the fixes sir.

@1awesomeJ
Copy link
Contributor Author

@bluca,
Good morning sir.
Please is there anything outstanding on this PR?
What are the next steps now that the IDs have been added?
Will those next steps be part of this PR?

@bluca bluca removed util-lib selinux smack please-review PR is ready for (re-)review by a maintainer labels Sep 1, 2023
@bluca bluca merged commit ad5db94 into systemd:main Sep 1, 2023
47 of 48 checks passed
@bluca
Copy link
Member

bluca commented Sep 1, 2023

@bluca, Good morning sir. Please is there anything outstanding on this PR? What are the next steps now that the IDs have been added? Will those next steps be part of this PR?

The catalog is published at https://cgit.freedesktop.org/systemd/systemd/plain/catalog/systemd.catalog
It is very crude for now, and just a single page, but a starting point would be to update the bsod tool to create a link to that page, and to print the message-id to look for.

The outreachy internship period is now over, so you are not expected to work anymore - you are of course welcome to keep contributing if you want to

@1awesomeJ
Copy link
Contributor Author

@bluca, Good morning sir. Please is there anything outstanding on this PR? What are the next steps now that the IDs have been added? Will those next steps be part of this PR?

The catalog is published at https://cgit.freedesktop.org/systemd/systemd/plain/catalog/systemd.catalog It is very crude for now, and just a single page, but a starting point would be to update the bsod tool to create a link to that page, and to print the message-id to look for.

The outreachy internship period is now over, so you are not expected to work anymore - you are of course welcome to keep contributing if you want to

I'd love to keep contributing.
I hope you're able to still guide me in your spare time sir.

Thank you.

@1awesomeJ
Copy link
Contributor Author

Now that the internship issue is closed,
Can I open a new issue with the subject "beginners seeking guidance" or so, so I can discuss with you there before I open PRs?
Sir?

@bluca
Copy link
Member

bluca commented Sep 1, 2023

Just open PRs directly, it's fine

@1awesomeJ
Copy link
Contributor Author

Okay sir.
When it's something I don't have an idea on how to get started? I'll open an "empty" PR, tag you, and wait for when you find time to respond.

Thank you sir.

@bluca
Copy link
Member

bluca commented Sep 1, 2023

You can use the mailing list for generic questions/topics https://lists.freedesktop.org/mailman/listinfo/systemd-devel

@1awesomeJ
Copy link
Contributor Author

You can use the mailing list for generic questions/topics https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Alright sir.

Although I feel not everyone may have the time and patience to attend to my questions which may be crude, but I'll try, and I'll tag you in PRs too sir.

YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Sep 4, 2023
YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[Outreachy 2023] Improve user experience on early system boot failures
2 participants