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

Fix plugin metrics #3552

Conversation

ArthurHlt
Copy link

Description

Plugin metrics store a CNSI Token with some metadata as we can see here those metadata are store as a k/v pairs.

But when those metadata need to be retrieve they are unmarshaled as a list of k/v pairs (see here ) which make this plugin unusable.

How Has This Been Tested?

This plugin doesn't include test, mainly just try to make works metrics as describe in the doc.

I was building myself jestream part, maybe it's change from golang 1.11 which break unmarshaling a "not a list" interface.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Docs update
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have followed the guidelines in CONTRIBUTING.md, including the required formatting of the commit message

@cfdreddbot
Copy link

✅ Hey ArthurHlt! The commit authors and yourself have already signed the CLA.

@ArthurHlt ArthurHlt changed the title Fix plugin metrics when retrieving metadata Fix plugin metrics May 1, 2019
@codecov
Copy link

codecov bot commented May 1, 2019

Codecov Report

Merging #3552 into v2-master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           v2-master   #3552      +/-   ##
============================================
- Coverage      51.83%   51.8%   -0.03%     
============================================
  Files            720     720              
  Lines          20221   20221              
  Branches        3612    3612              
============================================
- Hits           10481   10475       -6     
- Misses          9740    9746       +6

@KlapTrap
Copy link
Contributor

KlapTrap commented May 7, 2019

Ignore the snyk error, #3553 will fix it. @nwmac can you take a look at this PR please?

@KlapTrap KlapTrap requested a review from nwmac May 7, 2019 09:07
@richard-cox
Copy link
Contributor

@ ArthurHlt. Thanks for submitting this PR. I've done a quick investigation and it looks like we do store the TokenMetadata as an array

sqlite> select meta_data from tokens;
[
  {
    "type": "cf",  
    "url": "wss://doppler.local.pcfdev.io:443",
    "job": "cf-firehose"
  }
]

I also tried a version of the PR, with go 1.12, and got the following

log.Error("1: ")
// Parse out the metadata
var meta MetricsProviderMetadata
err := json.Unmarshal([]byte(endpoint.TokenMetadata), &meta)
log.Errorf("2: ", err)
if err == nil {
  info := MetricsMetadata{}
  info.EndpointGUID = endpoint.GUID
  info.Type = meta.Type
  info.URL = meta.URL
  info.Job = meta.Job
  info.Environment = meta.Environment
  metricsProviders = append(metricsProviders, info)
  log.Errorf("3: ", info)
}

ERRO[Wed May  8 16:40:58 BST 2019] 1:                                           
ERRO[Wed May  8 16:40:58 BST 2019] 2: %!(EXTRA *json.UnmarshalTypeError=json: cannot unmarshal array into Go value of type metrics.MetricsProviderMetadata)

My golang is terrible, so I'd like to understand the issue you're facing and the code a bit better. What's the error that you see when running v2-master? Could you possibly access your db and check the value of your meta_data token? Are you running new code against an old db (I need to check whether this value has always been an array or it's a recent thing)?

@richard-cox
Copy link
Contributor

@ArthurHlt Are you using bosh-prometheus and not stratos-metrics? If the former we might have fixed the root course for the issue you're seeing - #3602.

@ArthurHlt
Copy link
Author

Sorry i couldn't check for now, I am actually using bosh-prometheus yes.

Looks like #3602 fix this issue yes, this is a different approach :) probably because k8s part is giving a list

Thanks for the fixing and really really thanks for the job on stratos and scf. I'm closing this PR

@ArthurHlt ArthurHlt closed this May 24, 2019
@ArthurHlt ArthurHlt deleted the fix-plugin-metrics-metadata branch May 24, 2019 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants