Feat/replace identity in kedro telemetry with unique username

[jmholzer]

Description

Resolves #50

This PR assigns the hashed username of the Kedro user to the identity field in calls to the Heap API. This allows heap to aggregate data collected by the same user. A default username of 'anonymous' is assigned in the case that it cannot be determined. Currently, in this case, the identity is also set to 'anonymous'. This would cause Heap to pool together all 'anonymous' identities; it is unclear if this meets requirements, some feedback here would be good.

Development notes

The user's username is collected in before_command_run, using getpass.getuser() (the best cross-platform choice according to the pwd docs). It is then passed to _format_user_cli_data and _send_heap_event. Tests have been changed to reflect this.

Checklist

• Opened this PR as a 'Draft Pull Request' if it is work-in-progress
• Updated the documentation to reflect the code changes
• Added a description of this change in the relevant RELEASE.md file
• Added tests to cover my changes

[noklam]

      - unless:
          condition:
            or: 
              - and:
                - equal: [ "3.10", <<parameters.python_version>> ]
                - equal: [true, <<pipeline.parameters.run-build-kedro-datasets>>]
              - or:
                - equal: [false, <<pipeline.parameters.run-build-kedro-datasets>>]

The CI config has been modified and it works fine, but this is very hard to reason. I think it makes sense to refactor this into
this and make it a separate PR, can you try that?

      - when:
          condition:
            and: 
              - and:
                - equal: [true, <<pipeline.parameters.run-build-kedro-datasets>>]
              - or:
                - equal: ["3.7", <<parameters.python_version>>]
                - equal: ["3.8", <<parameters.python_version>>]
                - equal: ["3.9", <<parameters.python_version>>]

[merelcht]

This implementation looks good to me 🙂

On the point of grouping all "anonymous" events together: I think we should keep the same behaviour as before where if we can't determine the username we don't send any data at all. You've already switched to getpass.getuser() which is more robust then os.getlogin() afaik. But it would be good to hear what @yetudada thinks about this.

[merelcht]

Why did you remove this test?

[merelcht]

I noticed now you removed the timeout logic.

[yetudada]

This implementation looks good to me 🙂

On the point of grouping all "anonymous" events together: I think we should keep the same behaviour as before where if we can't determine the username we don't send any data at all. You've already switched to getpass.getuser() which is more robust then os.getlogin() afaik. But it would be good to hear what @yetudada thinks about this.

So I think following standup, it should be that if we can't determine the username they should get the unique Heap ID.

[jmholzer]

Perfect, I made this change in 7181e4d.

As I understand it from the heap docs, if the request is made with no identity field, heap automatically assigns the unique ID. In the case that no username can be found, the identity field is removed from the request.

[merelcht]

LGTM! 👍 Don't forget to update the release notes with this change.

[SajidAlamQB]

Pasting our DM here for clarity:

Regarding the telemetry ticket, your implementation looks good. Heap automatically assigns a User ID for each user and pools those users who have the same Identity into that User ID profile. So, I believe what Yetu meant in the case where we can't determine the Identity, we shouldn't set anything to Identity including "anonymous" and just let heaps automatic User ID pick it up. This seems to be the recommended method that heap suggests: https://developers.heap.io/docs/using-identify#identify-pitfalls

[SajidAlamQB]

LGTM, thank you!