Skip to content
This repository has been archived by the owner on Aug 31, 2022. It is now read-only.

Add dataset "device syslog" to non-db client (POC) #83

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

d-etienne
Copy link

@d-etienne d-etienne commented Jul 26, 2021

sonic-data-client/non-db client:
This PR aims to enable the streaming telemetry container to stream out the system logs from the SONiC Host. This dataset is added into non-database client since syslog messages are generated quite frequently.

  • Added syslog dataset path device/syslog
  • Added syslog data gathering function getDeviceSyslog()
  • Added On-Change functionality for the syslog data set

Need To Do:

  • Add the ability to identify whether the query that is being made by the client is actually a syslog query. Currently if the on-stream function is being used then the port will be opened

  • Add ability to identify how many subscribers are currently subscribed to the syslog data set. Currently the functionality is that when the subscriber cancels the subscription the port will close (This enables multiple clients to subscribe to this data set)

  • Add ability to compare the messages in a way that does not cause a massive delay when having two large messages. Currently, the messages are not being compared but rather all messages received through the port are placed within the linked list.

  • Adjust the functionality of polling and sampling queries for the syslog data set. As they no longer work with the changes that have been made to support on-change streaming mode

doc:
Added design document for the SONiC telemetry syslog pipeline describing implementation

How To Test:
We can use the command ./gnmi_cli -client_types=gnmi -a <DuT_IP>:8080 -t OTHERS -logtostderr -insecure -qt s -streaming_type ON_CHANGE -q device/syslog

Unit Tests will be added in a future iteration.

Signed-off-by: Daijah Etienne t-detienne@microsoft.com

@ghost
Copy link

ghost commented Jul 26, 2021

CLA assistant check
All CLA requirements met.

@d-etienne d-etienne closed this Jul 27, 2021
@d-etienne d-etienne reopened this Jul 27, 2021
@@ -229,6 +233,33 @@ func getCpuUtil() ([]byte, error) {
return b, nil
}

func getDeviceSyslog() ([]byte, error) {
hostName := "localhost"
portNum := "5150"
Copy link
Collaborator

Choose a reason for hiding this comment

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

please make it configurable parameter and pass in when the service start.

Copy link
Collaborator

Choose a reason for hiding this comment

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

you can have 5150 as default value.

Copy link
Collaborator

@hui-ma hui-ma left a comment

Choose a reason for hiding this comment

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

Given that this PR is not functional complete yet. Please mark it as POC in the title.

Please add in the description about what has been done and what are the to do items.

@@ -581,7 +679,7 @@ func (c *NonDbClient) Close() error {
return nil
}

func (c *NonDbClient) Set(delete []*gnmipb.Path, replace []*gnmipb.Update, update []*gnmipb.Update) error {
func (c *NonDbClient) Set(delete []*gnmipb.Path, replace []*gnmipb.Update, update []*gnmipb.Update) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what change did you make here?

@d-etienne d-etienne changed the title Add dataset "device syslog" to non-db client Add dataset "device syslog" to non-db client (POC) Aug 19, 2021
for {
elem, _ := getter()
// adds new elem to storage space
storageBuffer.PushBack(elem)
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you get elem as nil here and how do you handle it?

@@ -428,6 +473,10 @@ func (c *NonDbClient) StreamRun(q *queue.PriorityQueue, stop chan struct{}, w *s
for _, sub := range subscribe.GetSubscription() {
subMode := sub.GetMode()
if subMode != gnmipb.SubscriptionMode_SAMPLE {
if subMode == gnmipb.SubscriptionMode_ON_CHANGE {
Copy link
Collaborator

Choose a reason for hiding this comment

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

you need to change the comment of the StreamRun as well: "It supports SAMPLE mode only."

@@ -494,6 +543,55 @@ func streamSample(c *NonDbClient, stop chan struct{}, sub *gnmipb.Subscription,
}
}

func streamOnChange(c *NonDbClient, stop chan struct{}, sub *gnmipb.Subscription) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

does each subscriber have their owner streamOnChange? If how will you handle the case two client subscribe to syslog as the same time? have you tested it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you plan to enable onchange mode for other dataset? If not you should disable them in the call flow to avoid expected issue in the server.

@hui-ma
Copy link
Collaborator

hui-ma commented Aug 19, 2021

Please post test result. And also need to setup test case with constant syslog flow to test the timing in the pubsub to the storagebuffer.

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

Successfully merging this pull request may close these issues.

2 participants