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

Feature #4151551: TFS plugin refactor streamer.py file #278

Merged
merged 20 commits into from
Nov 27, 2024
Merged

Conversation

Miryam-Schwartz
Copy link
Contributor

@Miryam-Schwartz Miryam-Schwartz commented Nov 13, 2024

What

1. Create a seperate file for each class located in the streamer.py file
2. Remove the data fetching in the Prometheus format
3. Remove the inline args usage from the flow
4. Break the UFMTelemetryStreaming class into more granular classes

Why ?

https://redmine.mellanox.com/issues/4151551.
Remove unused functionality and break large file into smaller ones.

How ?

_
Break the UFMTelemetryStreaming class into three classes:

  1. TelemetryParser: Handles fetching the telemetry data, parsing, processing, and caching it in a map.
  2. TelemetryAttributesManager: Updates the saved/cached attributes (headers) when the telemetry data is parsed.
  3. Streamer: Streams data to Fluentd using either the HTTP protocol or the FORWARD protocol
    _

Testing ?

Regression tests for TFS

Special triggers

Use the following phrases as comments to trigger different runs

  • bot:retest rerun Jenkins CI (to rerun GitHub CI, use "Checks" tab on PR page and rerun all jobs)
  • bot:upgrade run additional update tests

@Miryam-Schwartz Miryam-Schwartz changed the title Tfs refactor Feature #4151551: TFS plugin refactor streamer.py file Nov 13, 2024
Copy link
Contributor

@ananalaghbar ananalaghbar left a comment

Choose a reason for hiding this comment

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

Well done, Miryam!
please see my initial commits, I still need to review the functionality.
and also, please make sure to set the correct git configurations, need to append the current commits to show your name instead of root

@ananalaghbar
Copy link
Contributor

@Miryam-Schwartz , general comment, please also review the current README file and make sure it doesn't contain any information about the removed functionalities , such as supporting the promethues format and inline args

@TalLerner TalLerner merged commit a31883f into main Nov 27, 2024
2 checks passed
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.

4 participants