Internal API - Visibility System support #1314
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 <[email protected]>
Co-authored-by: Michael Sverdlov <[email protected]>
Co-authored-by: Michael Sverdlov <[email protected]>
…jfrog-cli-core into visibility-system-usage
| @@ -747,6 +748,12 @@ func (serverDetails *ServerDetails) CreateAccessAuthConfig() (auth.ServiceDetail | |||
| return serverDetails.createAuthConfig(pAuth) | |||
| } | |||
|
|
|||
| 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 👍
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