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

Empty string inserting into Postgres blackouts table (should be NULL) #1543

Closed
mjtice opened this issue Jun 9, 2021 · 5 comments · Fixed by alerta/alerta-webui#507 or #1643
Closed
Labels
bug Something isn't working priority: medium A problem affecting a core component for which there is a workaround

Comments

@mjtice
Copy link

mjtice commented Jun 9, 2021

Issue Summary
I've been testing blackouts and I found that sometimes the blackout condition wasn't being met so a notification was being generated when it shouldn't have.

Looking through the is_blackout_period method it looks like the SELECT statement is expecting certain columns to be NULL.

I inspected the blackouts table in the database and found that the columns were empty strings vs NULL:
e.g.

postgres@[local]alertad=# \pset null 'NULLZ'
Null display is "NULLZ".
postgres@[local]alertad=# select * from blackouts;
-[ RECORD 1 ]-------------------------------------
id          | 34ae6ef1-9aaa-462c-9f1b-2c661bef7b7e
priority    | 6
environment | Production
service     | {}
resource    |
event       |
group       | NULLZ
tags        | {dbaautomation}
customer    | dbsa
start_time  | 2021-06-09 06:00:00
end_time    | 2021-06-09 14:15:00
duration    | 29700
user        | matt
create_time | 2021-06-09 13:11:32.45
text        |
origin      | NULLZ

which explains why the SELECT statement isn't being matched.

Environment

  • OS: Centos 7.9

  • API version: 8.5.0

  • Deployment: self-hosted

  • For self-hosted, WSGI environment: nginx/uwsgi

  • Database: Postgres 10.4

  • Server config:
    Auth enabled? Yes
    Auth provider? LDAP
    Customer views? Yes

  • web UI version: 8.5.0

  • CLI version: 8.5.0

To Reproduce
I was unable to reproduce this issue. I thought it had to do with me either copying or updating existing blackouts but further testing didn't result in the empty strings being inserted.

Expected behavior

Screenshots

Additional context
I think modifying the INSERT statement in the create_blackout method to evaluate an empty string as a NULL may be an appropriate "safety-check".

e.g.

    def create_blackout(self, blackout):
        insert = """
            INSERT INTO blackouts (id, priority, environment, service, resource, event,
                "group", tags, origin, customer, start_time, end_time,
                duration, "user", create_time, text)
            VALUES (%(id)s, %(priority)s, %(environment)s, %(service)s, nullif(%(resource)s,''), nullif(%(event)s,''),
                nullif(%(group)s,''), %(tags)s, nullif(%(origin)s,''), %(customer)s, %(start_time)s, %(end_time)s,
                %(duration)s, %(user)s, %(create_time)s, %(text)s)
            RETURNING *, duration AS remaining
        """
        return self._insert(insert, vars(blackout))

NOTE: Please provide as much information about your issue as possible.
Failure to provide basic details about your specific environment make
it impossible to know if an issue has already been fixed, can delay a
response and may result in your issue being closed without a resolution.

@satterly
Copy link
Member

If you can't reproduce the issue then it's not likely I'll be able to do it from the information you provided. For a start you don't say how you generated the blackout period. Which would be a pretty useful thing to know. ie. CLI, API, web UI? You say you conducted tests, but don't say what those tests were or what you did. So I'm not sure what you expect me to do.

@satterly satterly added the blocked This issue can't be resolved unless something else is done label Jun 10, 2021
@mjtice
Copy link
Author

mjtice commented Jun 10, 2021

I was creating the blackouts via the webui. I'm not really asking for anything - just mentioning that empty strings were being inserted into the database when they should have been NULLs.

I can work on trying to reproduce the issue. Based on my snippet above I can create a PR, too, if that would help.

@satterly
Copy link
Member

Did you click into the resource and/or event field of the "create blackout" dialog box but not input anything?

@mjtice
Copy link
Author

mjtice commented Jun 10, 2021

It looks like it's happening after I edit a blackout.

  1. Create a blackout. customer=dbsa, environment=Production, resource=myresource
# select * from blackouts ;
-[ RECORD 1 ]-------------------------------------
id          | 660f390b-5edd-4cec-8dde-ff58f3c7b48c
priority    | 2
environment | Production
service     | {}
resource    | myresource
event       | NULLZ
group       | NULLZ
tags        | {}
customer    | dbsa
start_time  | 2021-06-10 13:00:00
end_time    | 2021-06-10 14:00:00
duration    | 3600
user        | matt
create_time | 2021-06-10 12:47:33.652
text        |
origin      | NULLZ
  1. Edit the blackout. I remove the resource and add a tag=cdc
# select * from blackouts ;
-[ RECORD 1 ]-------------------------------------
id          | 660f390b-5edd-4cec-8dde-ff58f3c7b48c
priority    | 2
environment | Production
service     | {}
resource    |
event       | NULLZ
group       | NULLZ
tags        | {cdc}
customer    | dbsa
start_time  | 2021-06-10 13:00:00
end_time    | 2021-06-10 14:00:00
duration    | 3600
user        | matt
create_time | 2021-06-10 12:47:33.652
text        |
origin      | NULLZ
  1. resource is now an empty string.
  2. I just ran through adding (then updating to remove) event, and group and the same empty string (non-NULL) was inserted.

Side note:
One thing I found, too, is that if I edit the blackout and try to add the origin that value in the DB never updates.

  1. edit the blackout, add origin=myorigin
# select * from blackouts ;
-[ RECORD 1 ]-------------------------------------
id          | 660f390b-5edd-4cec-8dde-ff58f3c7b48c
priority    | 2
environment | Production
service     | {}
resource    |
event       | NULLZ
group       | NULLZ
tags        | {cdc}
customer    | dbsa
start_time  | 2021-06-10 13:00:00
end_time    | 2021-06-10 14:00:00
duration    | 3600
user        | matt
create_time | 2021-06-10 12:47:33.652
text        |
origin      | NULLZ

@satterly
Copy link
Member

Thanks for investigating further and providing additional info. This definitely sounds like a bug.

@satterly satterly added bug Something isn't working and removed blocked This issue can't be resolved unless something else is done labels Jun 10, 2021
@3IMOH 3IMOH added the priority: medium A problem affecting a core component for which there is a workaround label Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: medium A problem affecting a core component for which there is a workaround
Projects
Status: Done
3 participants