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

Store rack install & rack update telemetry data (V2) #3668

Closed
wants to merge 15 commits into from

Conversation

yuriadams
Copy link
Contributor

What is the feature/fix?

Screenshot 2023-04-25 at 11 14 26

Add screenshot or video (optional)

Doc: https://convox602-my.sharepoint.com/:w:/g/personal/yuri_adams_convox_com/ERKCC87MHOxLi6_3F8PqWqIBOWecg2nEg06tIbl9NA6ujQ?e=8bW5AX

Does it has a breaking change?

No

@yuriadams yuriadams marked this pull request as draft April 25, 2023 14:15
@yuriadams yuriadams marked this pull request as ready for review April 26, 2023 11:50
@yuriadams yuriadams added this to the 20230501 milestone May 1, 2023
@yuriadams yuriadams mentioned this pull request May 1, 2023
10 tasks
Twsouza
Twsouza previously approved these changes May 2, 2023
@yuriadams yuriadams modified the milestones: 20230501, 20230503 May 3, 2023
@yuriadams yuriadams mentioned this pull request May 3, 2023
10 tasks
provider/aws/formation/rack.json Outdated Show resolved Hide resolved
Comment on lines +25 to +27
"ApiCount",
"ApiCpu",
"ApiMonitorMemory",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should redacted this, it's a simple parameter and it doesn't contain any sensitive information.

"Version",
}, ",")

redactedParams = strings.Join([]string{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imho, the following parameters should not be redacted:
ApiRouter
ApiWebMemory
Autoscale
AvailabilityZones
BuildCpu
BuildImage
BuildInstance
BuildMemory
BuildVolumeSize
Development
EcsPollInterval
EncryptEbs
Encryption
HighAvailability
ImagePullBehavior
IMDSHttpTokens
Internal
InternalOnly
InstanceType
InstanceUpdateBatchSize
LogDriver
LogRetention
Private
PrivateApi
PrivateApiSecurityGroup
PrivateBuild
ScheduleRackScaleDown
ScheduleRackScaleUp
SpotInstanceBid
Subnet0CIDR
Subnet1CIDR
Subnet2CIDR
SubnetPrivate0CIDR
SubnetPrivate1CIDR
SubnetPrivate2CIDR
SwapSize
SyslogFormat
VPCCIDR
Tenancy

They don't have sensitive information and will be helpful for telemetry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ntner @ddoc, should keep all string params redacted or is it worth do not redacted this list?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although they are string, they are a list of values or just numbers.

provider/aws/workers_heartbeat.go Outdated Show resolved Hide resolved
@Twsouza Twsouza removed this from the 20230503 milestone May 8, 2023
@yuriadams yuriadams added this to the 20230509 milestone May 9, 2023
@yuriadams yuriadams mentioned this pull request May 9, 2023
10 tasks
@yuriadams yuriadams requested a review from Twsouza May 9, 2023 16:46
@yuriadams yuriadams modified the milestones: 20230509, 20230510 May 9, 2023
@yuriadams yuriadams mentioned this pull request May 10, 2023
10 tasks
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.

2 participants