Conversation
| type labels struct { | ||
| ProductID string `json:"product_id"` | ||
| FeatureID string `json:"feature_id"` | ||
| OIDCUsed string `json:"oidc_used"` | ||
| JobID string `json:"job_id"` | ||
| RunID string `json:"run_id"` | ||
| GitRepo string `json:"git_repo"` | ||
| GhTokenForCodeScanningAlertsProvided string `json:"gh_token_for_code_scanning_alerts_provided"` | ||
| } | ||
|
|
||
| type visibilityMetric struct { | ||
| Value int `json:"value"` | ||
| MetricsName string `json:"metrics_name"` | ||
| Labels labels `json:"labels"` | ||
| } |
There was a problem hiding this comment.
without omitempty it will send here Value 0 for empty value for example. If its a possible value a believe that its not good. same goes with empty strings.
with omitempty , it won't but it won't send actual 0 value if needed.
so it's need to be decided according to context.
There was a problem hiding this comment.
Thanks @sverdlov93.
Omit empty won't be good in the case for the following reasons:
- We'd like to send empty strings in the JSON payload, if no values are sent. I believe that the payload structure is expected to be consistent.
- The value that we sent is always 1, so it'll never be empty.
| Labels labels `json:"labels"` | ||
| } | ||
|
|
||
| func (vsm *VisibilitySystemManager) createMetric(commandName string) ([]byte, error) { |
There was a problem hiding this comment.
why dont we use VisibilityMetric as a return type, to enforce this type on the manager.LogMetric() func?
There was a problem hiding this comment.
That indeed makes sense @sverdlov93. Thanks!
Let me implement this in a follow-up PR, after this upcoming initial release.
Co-authored-by: Michael Sverdlov <sverdlov93@gmail.com>
Co-authored-by: Michael Sverdlov <sverdlov93@gmail.com>
Co-authored-by: Michael Sverdlov <sverdlov93@gmail.com>
…jfrog-cli-core into visibility-system-usage
|
|
||
| func (serverDetails *ServerDetails) CreateJfConnectAuthConfig() (auth.ServiceDetails, error) { | ||
| pAuth := accessAuth.NewAccessDetails() | ||
| pAuth.SetUrl(utils.AddTrailingSlashIfNeeded(serverDetails.Url) + "jfconnect/") |
There was a problem hiding this comment.
woudln't this cause double jfconnect/ on the URL?
because here you create the base URL ecosys.jfrog.io/access/jfconnect and on the PostMetric you add jfconnect/api/v1/backoffice/metrics/log
so wouldn't it become: ecosys.jfrog.io/access/jfconnect/jfconnect/api/v1/backoffice/metrics/log
There was a problem hiding this comment.
I think you're right @sverdlov93. Good catch 👍
3 participants
This pull request introduces enhancements to the usage reporting mechanism in the JFrog CLI by adding a new VisibilitySystemManager to log metrics and integrating it into existing commands
Depends on jfrog/jfrog-client-go#1060