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

CFE_TBL should send events instead of using syslog #607

Open
CDKnightNASA opened this issue Apr 14, 2020 · 5 comments
Open

CFE_TBL should send events instead of using syslog #607

CDKnightNASA opened this issue Apr 14, 2020 · 5 comments
Assignees

Comments

@CDKnightNASA
Copy link
Contributor

Is your feature request related to a problem? Please describe.
There's still a fair bit of code in CFE_TBL that sends SysLog messages rather than generating events.

Describe the solution you'd like
These messages should be removed and events generated where appropriate.

Requester Info
Christopher.D.Knight@nasa.gov

@CDKnightNASA CDKnightNASA linked a pull request Apr 21, 2020 that will close this issue
@skliper skliper added this to the 6.8.0 milestone May 12, 2020
@CDKnightNASA
Copy link
Contributor Author

Note that there are (still) 61 calls to CFE_ES_WriteToSysLog() in the CFE_TBL code. This will take some time to convert them all (probably ~50 new EID's, updates to TBL and UT.)

@skliper
Copy link
Contributor

skliper commented May 12, 2020

Cleared the milestone since further updates will likely miss the 6.8 cutoff (and not critical)

@skliper
Copy link
Contributor

skliper commented May 12, 2020

Removed link to #606 since it's not a full fix (and shouldn't close this issue once merged)

@skliper
Copy link
Contributor

skliper commented May 12, 2021

At least for CFE_TBL_Register I actually prefer how it's currently done, there's specific syslog writes for each possible error but one event at the end of the function if it's an error status (with status and table name). These are all debug sort of errors that are unlikely in an operational system unless there's dynamic registration or sharing. This avoids creating a huge number of events where filters are a limited resource, provides detailed info in the log, while also providing an overall event. Maybe more of the API's should follow this pattern?

@skliper
Copy link
Contributor

skliper commented Jun 16, 2021

In reviewing all the event id's and associated overloads (see #1588) it seems like we are due a re-evaluation of the current approach and suggested patterns. Maybe for each API that could error, accept an optional error event ID, if it's defined and there is an error then send the event with the error info (using SendEvent so the caller AppID is included). Could provide a legacy wrapper that retains former behavior (based on return code, SendEventWithAppID and the service defined event). Idea stems from a desire to provide a unique event id for commands, vs API calls from different contexts but yet share logic/implementation. Could combine with the "health" telemetry value concept so even if in the short event mode the last encountered error code could be reported. Right now it could be challenging to figure out the actual context of an event (especially in short mode).

The concept is trying to promote a more consolidated approach, where command handlers could really "own" all their event IDs and it would be clear from that context what could be produced by the command. Similar with apps utilizing APIs, vs the current approach with scattered, inconsistent event reporting with unclear context.

For an unspecified error Event ID write to the system log including calling context (app name). Could all be consistently implemented with a generic API providing standard fallback behavior and formatting. This would ensure errors are always reported somewhere and contain sufficient context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants