saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.17k stars 5.48k forks source link

carbon returner not using metric_base_pattern (not implemented) #43332

Open anitakrueger opened 7 years ago

anitakrueger commented 7 years ago

Description of Issue/Question

Despite the documentation saying that a metric pattern can be specified for the carbon configuration, it seems the functionality is not implemented. https://docs.saltstack.com/en/latest/ref/returners/all/salt.returners.carbon_return.html

Setup

carbon:
  host: graphite.example.com
  port: 2013
  skip_on_error: True
  mode: text
  metric_base_pattern: PREFIX.salt.[module].[function].[minion_id]

Steps to Reproduce Issue

I had added some debug statements to the carbon returner, but even the metric_base_pattern variable would be set to None in https://github.com/saltstack/salt/blob/develop/salt/returners/carbon_return.py#L232. I believe the line should actually be

    metric_base_pattern = opts.get('metric_base_pattern')

And then the attribute has to be added in _get_options in https://github.com/saltstack/salt/blob/develop/salt/returners/carbon_return.py#L111

    Returns options used for the carbon returner.
    '''
    attrs = {'host': 'host',
             'port': 'port',
             'skip': 'skip_on_error',
             'metric_base_pattern': 'metric_base_pattern',
             'mode': 'mode'}
...

And then last but not least, the functionality has to be implemented as to where the pattern is adhered to. I'm not sure how to do that right now, otherwise I'd submit a pull request.

Versions Report

Salt Version:
           Salt: 2017.7.1

Dependency Versions:
           cffi: Not Installed
       cherrypy: 3.2.2
       dateutil: 2.6.0
      docker-py: Not Installed
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 0.9.1
   msgpack-pure: 0.1.3
 msgpack-python: 0.4.6
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.6 (default, Oct 26 2016, 20:30:19)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.0.1
           RAET: Not Installed
          smmap: 0.8.2
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.4

System Versions:
           dist: Ubuntu 14.04 trusty
         locale: ANSI_X3.4-1968
        machine: x86_64
        release: 3.13.0-101-generic
         system: Linux
        version: Ubuntu 14.04 trusty
gtmanfred commented 7 years ago

@MrCitron could you take a look at this?

It looks like you initially implemented the metric base pattern stuff.

Unfortunately I do not know anything about this carbon returner.

Thanks! Daniel

MrCitron commented 7 years ago

Hi,

@gtmanfred : the purpose of this feature was to provide a way to customize the carbon "data path" when using the carbon returner. This way, you can then filter and made graphite/grafana boards easier.

@anitakrueger You can have a look at my original commit https://github.com/saltstack/salt/commit/f8b2f8079fc7c2c479c47e4864731f8ca2eb56a6#diff-6c5bda86baa432906f4144404c33987e and at the PR #25657

I had a quick look at the changes made after this commit, and I can not see anything obviously breaking the feature.

Hope this helps, Metin

anitakrueger commented 7 years ago

@MrCitron thanks a lot for providing the detail. It looks like this got merged into the 2015.5 branch and then it got lost in 2015.8 :( If you look at https://github.com/saltstack/salt/blob/2015.5/salt/returners/carbon_return.py, your code is there. And in https://github.com/saltstack/salt/blob/2015.8/salt/returners/carbon_return.py it isn't. It's also not in develop.

@gtmanfred Can you look at merging the code again by any chance?

Thanks a lot, Anita

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

anitakrueger commented 5 years ago

@gtmanfred @MrCitron Looks like nobody ever got this commit from 2015.5 to 2015.8. Any chance of getting these changes into one of the current branches?

stale[bot] commented 5 years ago

Thank you for updating this issue. It is no longer marked as stale.

gtmanfred commented 5 years ago

github says that commit is in all of these releases.

screen shot 2019-01-02 at 1 57 25 pm
gtmanfred commented 5 years ago

also, ping @saltstack/team-triage

anitakrueger commented 5 years ago

I don't understand it :( I'm looking at https://github.com/saltstack/salt/blob/2018.3/salt/returners/carbon_return.py and I don't see the metric_base_pattern being applied. I'm looking at https://github.com/saltstack/salt/blob/develop/salt/returners/carbon_return.py and I can't see it there either. My local installation is running 2018.3.2 and looking at the file /usr/lib/python2.7/site-packages/salt/returners/carbon_return.py all I can find is this in line 235:

    metric_base_pattern = opts.get('carbon.metric_base_pattern')

What am I missing?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

anitakrueger commented 4 years ago

Bit of a shame that this was working in 2015.5 and never got merged into master.

@jfindlay I don't know who to ping on this, any ideas please? I can't tag @saltstack/team-triage like @gtmanfred suggested.

stale[bot] commented 4 years ago

Thank you for updating this issue. It is no longer marked as stale.