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

✨Add simcore_user_agent and product_name to user service labels #3990

Merged
merged 24 commits into from
Mar 23, 2023

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented Mar 17, 2023

What do these changes do?

Extends the user services with new labels: simcore_user_agent and product_name.

Note simcore_user_agent will be empty or have the value puppeteer as seen below:

"X-Simcore-User-Agent": "puppeteer"

Related issue/s

How to test

Checklist

  • Unit tests for the changes exist
  • Runs in the swarm

@GitHK GitHK self-assigned this Mar 17, 2023
@GitHK GitHK added changelog:✨new-feature a:webserver issue related to the webserver service a:director-v2 issue related with the director-v2 service labels Mar 17, 2023
@GitHK GitHK added this to the Mithril milestone Mar 17, 2023
@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #3990 (f91eefc) into master (bb2b726) will decrease coverage by 0.1%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3990     +/-   ##
========================================
- Coverage    85.2%   85.2%   -0.1%     
========================================
  Files         858     874     +16     
  Lines       38061   39496   +1435     
  Branches      539     729    +190     
========================================
+ Hits        32446   33660   +1214     
- Misses       5490    5641    +151     
- Partials      125     195     +70     
Flag Coverage Δ
integrationtests 61.9% <60.0%> (-4.2%) ⬇️
unittests 82.7% <100.0%> (+0.3%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...service_director_v2/api/routes/dynamic_services.py 89.2% <ø> (ø)
...ector_v2/modules/dynamic_sidecar/scheduler/_abc.py 100.0% <ø> (ø)
...modules/dynamic_sidecar/scheduler/_core/_events.py 62.0% <ø> (-35.2%) ⬇️
...ules/dynamic_sidecar/scheduler/_core/_scheduler.py 76.5% <ø> (-2.2%) ⬇️
...ctor_v2/modules/dynamic_sidecar/scheduler/_task.py 94.1% <ø> (ø)
...ckages/models-library/src/models_library/docker.py 90.0% <100.0%> (+4.8%) ⬆️
...rvice-library/src/servicelib/aiohttp/monitoring.py 72.9% <100.0%> (ø)
...s/service-library/src/servicelib/common_headers.py 100.0% <100.0%> (ø)
...or_v2/models/schemas/dynamic_services/scheduler.py 97.3% <100.0%> (-2.0%) ⬇️
...v2/modules/dynamic_sidecar/docker_compose_specs.py 72.1% <100.0%> (-27.1%) ⬇️
... and 5 more

... and 184 files with indirect coverage changes

@GitHK GitHK marked this pull request as ready for review March 17, 2023 15:24
@GitHK GitHK requested a review from mrnicegyu11 March 17, 2023 15:24
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

I would use the very same label that already exists on the metrics:
simcore_user_agent

And also the header you check is the wrong one.

@GitHK GitHK changed the title ✨Add user_agent and product_name to user service labels ✨Add simcore_user_agent and product_name to user service labels Mar 20, 2023
@GitHK GitHK requested a review from sanderegg March 22, 2023 09:27
@GitHK GitHK enabled auto-merge (squash) March 23, 2023 09:27
@codeclimate
Copy link

codeclimate bot commented Mar 23, 2023

Code Climate has analyzed commit f91eefc and detected 0 issues on this pull request.

View more on Code Climate.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.1% 0.1% Duplication

@GitHK GitHK merged commit 0d96bd1 into ITISFoundation:master Mar 23, 2023
@GitHK GitHK deleted the pr-osparc-labels-and-agent branch April 4, 2023 12:08
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Apr 4, 2023
25 tasks
@elisabettai
Copy link
Collaborator

Hi @GitHK, I'm having a look in master if we can filter (dynamic) services started by puppeteers.
Where am I supposed to find the simcore_user_agent?
In Prometheus, I'm looking at metrics such as simcore_simcore_service_director_services_started_total and a typical returned entry is:

simcore_simcore_service_director_services_started_total{deployment="master", instance="10.11.6.159:8080", job="simcore", service_key="simcore/services/dynamic/raw-graphs", service_name="director", service_tag="2.11.2", service_type="DYNAMIC", user_id="undefined_user"}

I don't see any simcore_user_agent there and user_id is always "undefined_user". I was hoping user_id would be what we're looking for and thus being = puppeetteer in this case.
fyi @mrnicegyu11 and @sanderegg

@GitHK
Copy link
Contributor Author

GitHK commented Apr 11, 2023

@elisabettai this applies to dynamic-sidecar powered services. For legacy ones I am not aware if this should already be in place.

@elisabettai
Copy link
Collaborator

I see @GitHK, so the simcore_simcore_service_director_services_started_total give the legacy services, I haven't realized that, my bad. Still I find user_id=undefined_user on the director started services is a bit confusing. 😉

So now I'm looking at simcore_simcore_service_webserver_services_started_total on master, one example entry is:
simcore_simcore_service_webserver_services_started_total{deployment="master", instance="10.11.7.175:8080", job="simcore", service_key="simcore/services/dynamic/jupyter-smash", service_name="webserver", service_tag="3.0.7"}

And I don't see any simcore_user_agent. Would you know which metric has that?

@GitHK
Copy link
Contributor Author

GitHK commented Apr 11, 2023

@mrnicegyu11 might be able to answer your question there. I think the is currently dealing with it.

@GitHK
Copy link
Contributor Author

GitHK commented Apr 12, 2023

@elisabettai you should now be able to find the new label (for legacy services) in master as soon as the above PR gets deployed

@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Apr 20, 2023
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:director-v2 issue related with the director-v2 service a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants