Fetch locale information for Telemetry

[dharmit]

What type of PR is this?

/kind feature

What does this PR do / why we need it:
$subject

Which issue(s) this PR fixes:

Fixes part of #4994

PR acceptance criteria:

• Unit test

• Integration test

• Documentation

• I have read the test guidelines

How to test changes / Special notes to the reviewer:
Look at the data being captured in Segment. You should see "locale" information in the "Raw" data.

[kadel]

Not sure why, but I don't see any locale data in segment console.

In order to have this as one of the traits you should it to following function as well https://github.com/openshift/odo/blob/44c35d5390ff0979310df44680619475cec9a871/pkg/segment/segment.go#L144-L147

[dharmit]

Not sure why, but I don't see any locale data in segment console.

Did you check in the "Raw" tab? Even the time zone data that's added via #5067 shows in the "Raw" tab in context. I'm curious to understand why we're using traits when context struct already provides keys of our interest:

// This type provides the representation of the `context` object as defined in
// https://segment.com/docs/spec/common/#context
type Context struct {
	App       AppInfo      `json:"app,omitempty"`
	Campaign  CampaignInfo `json:"campaign,omitempty"`
	Device    DeviceInfo   `json:"device,omitempty"`
	Library   LibraryInfo  `json:"library,omitempty"`
	Location  LocationInfo `json:"location,omitempty"`
	Network   NetworkInfo  `json:"network,omitempty"`
	OS        OSInfo       `json:"os,omitempty"`    <------------------------------
	Page      PageInfo     `json:"page,omitempty"`
	Referrer  ReferrerInfo `json:"referrer,omitempty"`
	Screen    ScreenInfo   `json:"screen,omitempty"`
	IP        net.IP       `json:"ip,omitempty"`
	Locale    string       `json:"locale,omitempty"`    <------------------------------
	Timezone  string       `json:"timezone,omitempty"`    <------------------------------
	UserAgent string       `json:"userAgent,omitempty"`
	Traits    Traits       `json:"traits,omitempty"`

	// This map is used to allow extensions to the context specifications that
	// may not be documented or could be introduced in the future.
	// The fields of this map are inlined in the serialized context object,
	// there is no actual "extra" field in the JSON representation.
	Extra map[string]interface{} `json:"-"`
}

[kadel]

I'm curious to understand why we're using traits when context struct already provides keys of our interest:

Have you been able to get to the context data from Woopra? I was not able to see it there.

If you check how vscode telemetry does it, they send locale and timezone info in both context and traits. I assume that the reason is that you can't see the all the context data in Woopra.

Location might be a different story, that one is send only in context and it shows up in Woopra. I think that it might be because Woopra has a "native" location field, but no standard fieds for timezone and locale

[dharmit]

Have you been able to get to the context data from Woopra? I was not able to see it there.

Woopra is not making much sense to me, tbh. In the sense, I'm not good at using it, yet.

If you check how vscode telemetry does it, they send locale and timezone info in both context and traits. I assume that the reason is that you can't see the all the context data in Woopra.

Yeah, I assumed the same that adding it to traits could be for Woopra. I can add it to traits as well in the same PR if it helps.

[kadel]

Yeah, I assumed the same that adding it to traits could be for Woopra. I can add it to traits as well in the same PR if it helps.

yes please

[kadel]

If locale detection fails, the whole telemetry will fail, and nothing will be send, do we want that?

[dharmit]

Snap. I did it correctly when I added locale info in traits. Fixed now.

[feloy]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: feloy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [feloy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-bot]

/retest-required

Please review the full test history for this PR and help us cut down flakes.

[feloy]

@dharmit It seems that %w is not supported by Infof

pkg/segment/segment.go:171:3: Infof call has error-wrapping directive %w

[dharmit]

@dharmit It seems that %w is not supported by Infof

pkg/segment/segment.go:171:3: Infof call has error-wrapping directive %w

I think I should revert it to the way it was, that is, use err.Error() in that call. WDYT?

[feloy]

@dharmit It seems that %w is not supported by Infof

pkg/segment/segment.go:171:3: Infof call has error-wrapping directive %w

I think I should revert it to the way it was, that is, use err.Error() in that call. WDYT?

Yes, and also using %s instead of %w I think

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[openshift-ci]

@dharmit: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/psi-k8s-ibmc-integration-e2e	8388a73	link	false	/test psi-k8s-ibmc-integration-e2e

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[feloy]

/lgtm