added: display last db update whenever trivy server is started

[yashvardhan-kukreja]

#346
Results before the change
trivy server --listen localhost:8080
2020-06-23T10:28:45.072+0530 INFO Listening localhost:8080...

Results after the change
trivy server --listen localhost:8080
2020-06-23T10:28:45.072+0530 INFO Last DB Update at: 2020-06-23 00:18:04.500501189 +0000 UTC
2020-06-23T10:28:45.072+0530 INFO Listening localhost:8080...

[knqyf263]

I think #346 is talking about Prometheus metrics, not log messages.

[yashvardhan-kukreja]

Ohh, I might have gotten confused. I have a few basic doubts with the issue which I have mentioned in the thread of the issue, please clarify them

[knqyf263]

OK. Let's wait for @computeralex92's answer.

[yashvardhan-kukreja]

Hi @knqyf263, @computeralex92! I added prometheus metrics endpoint support and if you go to GET /metrics, you'd get the following output:

# HELP trivy Gauge Metrics associated with trivy - Last DB Update, Last DB Update Attempt ...
# TYPE trivy gauge
trivy{action="last_db_update"} 1.593432599e+09
trivy{action="last_db_update_attempt"} 1.593432599e+09

The timestamps, here, are being stored as Unix Timestamps (float64) because that's what the prometheus Gauge expects.
Also, here, I used prometheus Gauge for storing the values (timestamps) for these labels because the prometheus Counters were restrictive and rough.

I hope this seems good and if it does then, we can go on and implement the remaining three metrics suggested by @computeralex92.

[computeralex92]

Hi @knqyf263, @computeralex92! I added prometheus metrics endpoint support and if you go to GET /metrics, you'd get the following output:

# HELP trivy Gauge Metrics associated with trivy - Last DB Update, Last DB Update Attempt ... # TYPE trivy gauge trivy{action="last_db_update"} 1.593432599e+09 trivy{action="last_db_update_attempt"} 1.593432599e+09

The timestamps, here, are being stored as Unix Timestamps (float64) because that's what the prometheus Gauge expects.
Also, here, I used prometheus Gauge for storing the values (timestamps) for these labels because the prometheus Counters were restrictive and rough.

I hope this seems good and if it does then, we can go on and implement the remaining three metrics suggested by @computeralex92.

Looks great; I looking forward to use this metrics ;-)

[yashvardhan-kukreja]

hi @simar7 , can u please review my PR

[simar7]

Left some comments. At the moment this is also not unit-tested. You could take a look at a few examples here: https://github.com/aquasecurity/trivy/blob/17b84f6c0902172938f8f83420dc432fa87f467e/pkg/rpc/server/listen_test.go

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[yashvardhan-kukreja]

So @simar7 , if such kind of situation happens, then should I programme the code to panic and exit with status 1. Or should I just programme it to log in the terminal "GaugeMetric not initialised" but still not make the code exit and hence, allow the user to use the existing session for other functionalities, if they want?

@simar7 , I am done with rest of all the issues you mentioned above, so, as soon as the above quoted doubt gets clarified, I'll push all the new changes to this PR 😄

[codecov]

Codecov Report

Merging #542 (0d7f4c2) into main (a00d719) will decrease coverage by 6.51%.
The diff coverage is 49.33%.

@@            Coverage Diff             @@
##             main     #542      +/-   ##
==========================================
- Coverage   68.50%   61.98%   -6.52%     
==========================================
  Files          57       65       +8     
  Lines        2210     2594     +384     
==========================================
+ Hits         1514     1608      +94     
- Misses        564      846     +282     
- Partials      132      140       +8     

Impacted Files	Coverage Δ
pkg/commands/artifact/config.go	81.81% <ø> (ø)
pkg/commands/artifact/fs.go	0.00% <0.00%> (ø)
pkg/commands/artifact/image.go	0.00% <0.00%> (ø)
pkg/commands/artifact/repository.go	0.00% <0.00%> (ø)
pkg/commands/artifact/run.go	0.00% <0.00%> (ø)
pkg/commands/artifact/wire_gen.go	0.00% <0.00%> (ø)
pkg/commands/client/config.go	79.31% <ø> (ø)
pkg/commands/client/run.go	0.00% <0.00%> (ø)
pkg/commands/client/wire_gen.go	0.00% <0.00%> (ø)
pkg/commands/config/artifact.go	55.00% <ø> (ø)
... and 43 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8935aa6...0d7f4c2. Read the comment docs.

[simar7]

@yashvardhan-kukreja can you try to rebase this on the latest master and resolve the git conflicts?

[simar7]

@yashvardhan-kukreja I'm still seeing a diff that shouldn't be there in the Dockerfile. See below:

Can you also try to run go mod tidy and re-commit the go.mod and go.sum files again if there are any changes?

[simar7]

lgtm, thanks for the contribution! @knqyf263 would you like to review before we merge?

[knqyf263]

@simar7 Yes, I'll take a look, but do you know why codecov is failing?

[simar7]

@simar7 Yes, I'll take a look, but do you know why codecov is failing?

I tried looking but I'm not sure. The failure doesn't make sense. Maybe it's a bug: https://docs.codecov.io/docs/unexpected-coverage-changes#think-its-codecovs-fault We shouldn't wait for that in this PR though. We can take a look it in a separate discussion.

[yashvardhan-kukreja]

@simar7 @knqyf263 , if the PR is at a merge-able point, should I go on squash all the above commits?

[knqyf263]

@yashvardhan-kukreja Basically, we merge PR with squash commits on GitHub, so you don't have to do that. But we probably should do that this time in order to resolve the error from codecov.

[yashvardhan-kukreja]

Went off the grid for a while, hence, the changes took a bit of time. sorry for that pause.
@knqyf263 - can u please review the changes

[simar7]

Went off the grid for a while, hence, the changes took a bit of time. sorry for that pause.
@knqyf263 - can u please review the changes

hi @yashvardhan-kukreja – @knqyf263 is out this week. I'll take a look at your new changes in the meantime.

[yashvardhan-kukreja]

@simar7 , no worries. And thanks for reviewing this meanwhile :)
PS: I have applied the changes as suggested

[yashvardhan-kukreja]

@knqyf263 @simar7 , any heads up/updates regarding this? Been a while 😅

[yashvardhan-kukreja]

@knqyf263 @simar7 I applied all the suggested changes, rebased them with the main branch and all the tests are successfully running.
Please review the above PR.

[yashvardhan-kukreja]

ping @knqyf263

[krol3]

@yashvardhan-kukreja, could you review the conflicting files?

[yashvardhan-kukreja]

@yashvardhan-kukreja, could you review the conflicting files?

I have rebased this PR with the latest state main. Sorry that it took some time. Was a little swamped and the conflicts were pretty major with respect to the new state of main branch :P

[yashvardhan-kukreja]

Also, I have added unit tests around the "extended config" structs as well for the sake of testing the Prometheus MetricsRegistry Setup as well

[yashvardhan-kukreja]

is this being reviewed @krol3 @knqyf263 ?

[krol3]

@simar7 @knqyf263 could you take a look on this PR?

[knqyf263]

I'm sorry for the late reply. I'm busy on a new feature scanning config files now. After wrapping up it, I'll review it.

[jc16180]

Is anyone working on this?

[knqyf263]

Instead, we've added /version endpoint.
#4869