Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

METRON-1599 Allow user to define global property 'source.type.field' in Ambari #1047

Closed
wants to merge 6 commits into from

Conversation

nickwallen
Copy link
Contributor

@nickwallen nickwallen commented Jun 4, 2018

The user can specify the name of the message field containing the source type in the global configuration. This is used by the Alerts UI.

Currently this necessitates a user using the CLI to add the field. This property should be exposed to the user via Ambari. The user should be able to define this property directly in Ambari.

Testing

  1. Launch the development environment. Ensure alerts are visible in the Alerts UI and that the Service Check passes.

  2. Open the REPL and validate the current value of the global property 'source.type.field'. The value here, the default value should be source:type.

    [root@node1 ~]# source /etc/default/metron
    [root@node1 ~]# $METRON_HOME/bin/stellar -z $ZOOKEEPER
    Stellar, Go!
    ...
    [Stellar]>>> globals := CONFIG_GET("GLOBAL")
    {
      ...
      "source.type.field" : "source:type",
      ...
    }
    
  3. Change the value in Ambari by going to Metron > Configs > REST > Source Type Field Name.

    screen shot 2018-06-18 at 2 33 11 pm

  4. After saving the change, Ambari should prompt for the following services to be restarted.

    1 Metron Alerts UI, 1 Metron Management UI, 1 Metron REST

    screen shot 2018-06-18 at 2 34 57 pm

  5. Restart all affected services.

  6. After the services have been restarted, ppen the REPL and validate that the value of the global property 'source.type.field' changed in the global config.

    [Stellar]>>> globals := CONFIG_GET("GLOBAL")
    {
      ...
      "source.type.field" : "different:source:type:field",
      ...
    }
    

Pull Request Checklist

  • Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
  • Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
  • Has your PR been rebased against the latest commit within the target branch (typically master)?
  • Have you included steps to reproduce the behavior or problem that is being changed or addressed?
  • Have you included steps or a guide to how the change may be verified and tested manually?
  • Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
  • Have you written or updated unit tests and or integration tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?

<name>source_type_field</name>
<display-name>Source Type Field</display-name>
<description>Name of the message field that contains the source type.</description>
<value>source.type</value>
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the default be "source:type"?

Copy link
Member

Choose a reason for hiding this comment

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

Currently the default in the UI, if unspecified, is source:type, so I'd vote keeping it as that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I was a little confused by this. I set it to source.type based on the value defined in Constants. The Enrichment and Indexing topologies use MessageUtils.getSensorType(msg) to get the source type of a message. Right now this will always look for a field called source.type.

But after digging a little more, it seems that this setting is currently only intended to define where the UI looks for the source type in the search indices, not to where Enrichment or Indexing look. That explains my confusion.

I will update this to use source:type as you guys have mentioned and also update the description to clarify that this field is only intended for the UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might also update this so the field only shows up under the "Advanced" settings, since it is not something we expect the user to have to, nor want to change in most cases. I think I can do that easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I addressed this by changing the default to source:type and improving the description in the latest commit.

@@ -137,4 +137,10 @@
<value>yyyy.MM.dd.HH</value>
<display-name>Elasticsearch Date Format</display-name>
</property>
<property>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think It might be more appropriate to have this property in metron-rest-env instead? Now when I change it I'm prompted to restart all Metron components when really the only one that needs to be restarted is REST and the UIs. Both UIs depend on REST so they will also be included in the restart list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, assuming this field is applicable only to the REST/UI/DAO, then this makes complete sense. Will address this.

from params import params
env.set_params(params)

metron_service.refresh_configs(params)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we move the property to metron-rest-env then I'm not sure this is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The call to refresh_configs is the only thing that pushes changes to the global config. I don't see how the setting would be pushed to the global config without it. Let me know if I am misunderstanding something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct, of course. Doh. I'll drop this.

@merrimanr
Copy link
Contributor

I tested this in full dev and it worked as advertised. +1

@asfgit asfgit closed this in 7427348 Jun 19, 2018
@nickwallen nickwallen deleted the METRON-1599 branch September 17, 2018 19:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants