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.

Sign up for GitHub

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

Jump to bottom

Feature: Add silence datasource #403

Open

dannyswebb wants to merge 9 commits into ansible-collections:main from dannyswebb:feat/add_silence_datasource

dannyswebb commented Oct 4, 2024

SUMMARY

Grafana has the ability to support multiple alertmanager datasources. Add the ability for grafana_silence to use user defined datasources (or default to grafana if none supplied).

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

grafana_silence

ADDITIONAL INFORMATION

n/a

danny-s-webb added 2 commits

October 4, 2024 09:26


          adding ability to target alertmanager datasources for silence creatio…

4c7340c

…n / deletion


          add tests for targeting alertmanager datasource for silences

a13d474

dannyswebb requested review from rrey and seuf as code owners

October 4, 2024 08:33

Collaborator

Nemental commented Oct 4, 2024

Hey @dannyswebb thank you for your PR :)
Can you please add a changelog fragment and check the results of the failed workflows?

codecov bot commented Oct 4, 2024 •

edited

Loading

Codecov Report

Attention: Patch coverage is 42.85714% with 8 lines in your changes missing coverage. Please review.

Project coverage is 70.64%. Comparing base (53c9402) to head (4e0cbc1).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
plugins/modules/grafana_silence.py	42.85%	7 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #403       +/-   ##
===========================================
+ Coverage   45.46%   70.64%   +25.17%     
===========================================
  Files          21       11       -10     
  Lines        2228     1468      -760     
  Branches      437      326      -111     
===========================================
+ Hits         1013     1037       +24     
+ Misses       1174      257      -917     
- Partials       41      174      +133

Flag	Coverage Δ
integration	`70.57% <42.85%> (?)`
sanity	`?`
units	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

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

danny-s-webb added 2 commits

October 4, 2024 14:46


          add changelog fragment

88b46d0


          fix documentation lint

539d502

dannyswebb force-pushed the feat/add_silence_datasource branch from b9fa5e5 to 539d502 Compare

October 4, 2024 13:46

Collaborator

Nemental commented Oct 8, 2024

Please use ruff and black for code linting/formatting.
https://github.com/ansible-collections/community.grafana?tab=readme-ov-file#contributing
Also there are some unit and sanity findings. Please take a look :) Thanks!

danny-s-webb added 4 commits

October 9, 2024 08:44


          add mocks for datasource_by_name

fd004e1


          black formatting

e59c96a


          fix mock ordering

d29a845


          remove extraneous newline

1dcea66

dannyswebb force-pushed the feat/add_silence_datasource branch from 89f2e39 to 1dcea66 Compare

October 9, 2024 09:02

Author

dannyswebb commented Oct 9, 2024

Hey @dannyswebb thank you for your PR :) Can you please add a changelog fragment and check the results of the failed workflows?

fragment added, working through the failed workflows now. Sorry, been a couple years since I've touched mock tests and I'm a bit rusty.

Author

dannyswebb commented Oct 9, 2024

Please use ruff and black for code linting/formatting. https://github.com/ansible-collections/community.grafana?tab=readme-ov-file#contributing Also there are some unit and sanity findings. Please take a look :) Thanks!

ruff and black linting done, working through the unit findings now.

Nemental requested changes

View reviewed changes

Collaborator

Nemental left a comment

I've added a few suggestions. If grafana is defined as the default alertmanager in the documentation area, this must also be set in the code. I also made a few renaming changes. My code suggestions are not tested.

plugins/modules/grafana_silence.py

@@ @@ -69,6 +69,12 @@ @@
                   type: list
                   elements: dict
                   required: true
+                alertmanager_datasource:
+                  description:
+                    - Which alertmanager datasource to target

Collaborator

Nemental Oct 10, 2024

Suggested change

      
                  - Which alertmanager datasource to target
          
                  - Which alertmanager datasource to target.

plugins/modules/grafana_silence.py

@@ @@ -352,6 +387,7 @@ def setup_module_object(): @@
                   org_name=dict(type="str"),
                   skip_version_check=dict(type="bool", default=False),
                   starts_at=dict(type="str", required=True),
+                  alertmanager_datasource=dict(type=str),

Collaborator

Nemental Oct 10, 2024

Suggested change

      
                alertmanager_datasource=dict(type=str),
          
                alertmanager_datasource=dict(type=str, default="grafana"),

plugins/modules/grafana_silence.py

Comment on lines +252 to +255

+                      if module.params.get("alertmanager_datasource", None):
+                          self.alertmanager_path = self.datasource_by_name(
+                              module.params["alertmanager_datasource"]
+                          )

Collaborator

Nemental Oct 10, 2024

Suggested change

      
                    if module.params.get("alertmanager_datasource", None):
          
                        self.alertmanager_path = self.datasource_by_name(
          
                            module.params["alertmanager_datasource"]
          
                        )
          
                    if module.params.get("alertmanager_datasource") != "grafana":
          
                        self.alertmanager_id = self.datasource_by_name(
          
                            module.params["alertmanager_datasource"]
          
                        )

plugins/modules/grafana_silence.py

Comment on lines +221 to +222

		# Default here because you can't look up "grafana" DS via API
		self.alertmanager_path = "grafana"

Collaborator

Nemental Oct 10, 2024

Suggested change

      
                    # Default here because you can't look up "grafana" DS via API
          
                    self.alertmanager_path = "grafana"
          
                    self.alertmanager_id = module.params["alertmanager_datasource"]

plugins/modules/grafana_silence.py

    
            @@ -280,7 +319,7 @@ def get_version(self):
          
                      raise GrafanaError("Failed to retrieve version from '%s'" % url)

                  def create_silence(self, comment, created_by, starts_at, ends_at, matchers):

                      url = "/api/alertmanager/grafana/api/v2/silences"

                      url = f"/api/alertmanager/{self.alertmanager_path}/api/v2/silences"

Collaborator

Nemental Oct 10, 2024

Suggested change

      
                    url = f"/api/alertmanager/{self.alertmanager_path}/api/v2/silences"
          
                    url = f"/api/alertmanager/{self.alertmanager_id}/api/v2/silences"

plugins/modules/grafana_silence.py

    
            @@ -297,7 +336,7 @@ def create_silence(self, comment, created_by, starts_at, ends_at, matchers):
          
                      return response

                  def get_silence(self, comment, created_by, starts_at, ends_at, matchers):

                      url = "/api/alertmanager/grafana/api/v2/silences"

                      url = f"/api/alertmanager/{self.alertmanager_path}/api/v2/silences"

Collaborator

Nemental Oct 10, 2024

Suggested change

      
                    url = f"/api/alertmanager/{self.alertmanager_path}/api/v2/silences"
          
                    url = f"/api/alertmanager/{self.alertmanager_id}/api/v2/silences"

plugins/modules/grafana_silence.py

-                      url = "/api/alertmanager/grafana/api/v2/silence/{SilenceId}".format(
-                          SilenceId=silence_id
-                      )
+                      url = f"/api/alertmanager/{self.alertmanager_path}/api/v2/silence/{silence_id}"

Collaborator

Nemental Oct 10, 2024

Suggested change

      
                    url = f"/api/alertmanager/{self.alertmanager_path}/api/v2/silence/{silence_id}"
          
                    url = f"/api/alertmanager/{self.alertmanager_id}/api/v2/silence/{silence_id}"

plugins/modules/grafana_silence.py

    
                      response = self._send_request(url, headers=self.headers, method="GET")

                      return response

                  def get_silences(self):

                      url = "/api/alertmanager/grafana/api/v2/silences"

                      url = f"/api/alertmanager/{self.alertmanager_path}/api/v2/silences"

Collaborator

Nemental Oct 10, 2024

Suggested change

      
                    url = f"/api/alertmanager/{self.alertmanager_path}/api/v2/silences"
          
                    url = f"/api/alertmanager/{self.alertmanager_id}/api/v2/silences"

plugins/modules/grafana_silence.py

-                      url = "/api/alertmanager/grafana/api/v2/silence/{SilenceId}".format(
-                          SilenceId=silence_id
-                      )
+                      url = f"/api/alertmanager/{self.alertmanager_path}/api/v2/silence/{silence_id}"

Collaborator

Nemental Oct 10, 2024

Suggested change

      
                    url = f"/api/alertmanager/{self.alertmanager_path}/api/v2/silence/{silence_id}"
          
                    url = f"/api/alertmanager/{self.alertmanager_id}/api/v2/silence/{silence_id}"


          Merge branch 'main' into feat/add_silence_datasource

4e0cbc1

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

Labels

None yet