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

draft: RFC5424 origin structure added #479

Open
wants to merge 1 commit into
base: latest
Choose a base branch
from

Conversation

vasiliyk
Copy link
Contributor

@vasiliyk vasiliyk commented Nov 17, 2024

draft: RFC5424 origin structure added; addressing #225

What I don't like:

  1. default stumpless_element_to_string function returns the origin structure in an incorrect format. Correct format should be: [origin ip="example.ip" software="stumpless" swVersion="example.version"]. Should I override function to account for it?
  2. stumpless syslog and vsyslog replacements don't return origin structure by default. Should an option be added to them to include origin structure?

Copy link

codecov bot commented Nov 17, 2024

Codecov Report

Attention: Patch coverage is 0% with 54 lines in your changes missing coverage. Please review.

Project coverage is 89.13%. Comparing base (f3ac58e) to head (ac9cef1).

Files with missing lines Patch % Lines
src/log.c 0.00% 54 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           latest     #479      +/-   ##
==========================================
- Coverage   90.22%   89.13%   -1.10%     
==========================================
  Files          47       47              
  Lines        4410     4464      +54     
  Branches      592      604      +12     
==========================================
  Hits         3979     3979              
- Misses        288      342      +54     
  Partials      143      143              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@goatshriek goatshriek left a comment

Choose a reason for hiding this comment

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

This will be quite a difficult addition, since there isn't an example/base set up for automatic elements yet.

The code to gather the IP addresses will need to be put into configuration source files and chosen based on the headers available, rather than the availability of WIN32. You can look at include/private/config/wrapper/network_supported.h to see the logic for this now. It would probably make the most sense to add to this, although the use of ifaddrs.h may warrant it's on header check and inclusion.

The automatic elements should be configurable, on by default, and not do too much special work apart from just creating and adding the element to entries before they're logged. For now, maybe adding a boolean flag to the target structure on whether or not automatic elements are enabled and then doing the extra work to add the element in stumpless_add_entry, or even better in format_entry is the best first step. This will allow you to bypass the concerns you note about the to_string format being incorrect, and the other syslog and vsyslog functions will still pick up these changes as well.

Also, be sure that you have some sort of test so that elements with the automatic element can be verified to have the correct data.

@goatshriek goatshriek self-assigned this Nov 17, 2024
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.

2 participants