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

Allow access to MetricSet and improve memory allocation on MetricSample #1236

Closed
wants to merge 3 commits into from

Conversation

glucaci
Copy link
Contributor

@glucaci glucaci commented Mar 22, 2021

For users to send custom metrics, must have access to MetricSet as in #322 mentioned.
In the same time I did an optimization on MetricSample.

I'm working o a prototype to integrate IPayloadSender.QueueMetrics with Metrics.NET library

@apmmachine
Copy link
Contributor

apmmachine commented Mar 22, 2021

❕ Build Aborted

The PR is not allowed to run in the CI yet

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts

Expand to view the summary

Build stats

  • Build Cause: Branch indexing

  • Reason: The PR is not allowed to run in the CI yet

  • Start Time: 2021-03-25T02:20:24.004+0000

  • Duration: 3 min 58 sec

  • Commit: 125b0cd

Trends 🧪

Image of Build Times

Steps errors 2

Expand to view the steps failures

Load a resource file from a shared library
  • Took 0 min 0 sec . View more details on here
  • Description: approval-list/elastic/apm-agent-dotnet.yml
Error signal
  • Took 0 min 0 sec . View more details on here
  • Description: githubPrCheckApproved: The PR is not allowed to run in the CI yet. (Only users with write permissions can do so.)

Log output

Expand to view the last 100 lines of log output

[2021-03-25T02:22:57.983Z] Running in /var/lib/jenkins/workspace/net_apm-agent-dotnet-mbp_PR-1236/apm-agent-dotnet
[2021-03-25T02:22:58.003Z] [INFO] gitCheckout: Checkout SCM PR-1236 with default customisation from the Item.
[2021-03-25T02:22:58.030Z] [INFO] Override default checkout
[2021-03-25T02:22:58.073Z] Sleeping for 10 sec
[2021-03-25T02:23:08.104Z] The recommended git tool is: git
[2021-03-25T02:23:08.243Z] using credential f6c7695a-671e-4f4f-a331-acdce44ff9ba
[2021-03-25T02:23:08.261Z] Wiping out workspace first.
[2021-03-25T02:23:08.271Z] Cloning the remote Git repository
[2021-03-25T02:23:08.271Z] Using shallow clone with depth 3
[2021-03-25T02:23:08.271Z] Avoid fetching tags
[2021-03-25T02:23:08.287Z] Cloning repository [email protected]:elastic/apm-agent-dotnet.git
[2021-03-25T02:23:08.317Z]  > git init /var/lib/jenkins/workspace/net_apm-agent-dotnet-mbp_PR-1236/apm-agent-dotnet # timeout=10
[2021-03-25T02:23:08.325Z] Fetching upstream changes from [email protected]:elastic/apm-agent-dotnet.git
[2021-03-25T02:23:08.325Z]  > git --version # timeout=10
[2021-03-25T02:23:08.331Z]  > git --version # 'git version 2.17.1'
[2021-03-25T02:23:08.331Z] using GIT_SSH to set credentials GitHub user @elasticmachine SSH key
[2021-03-25T02:23:08.335Z]  > git fetch --no-tags --progress -- [email protected]:elastic/apm-agent-dotnet.git +refs/heads/*:refs/remotes/origin/* # timeout=15
[2021-03-25T02:23:09.464Z] Cleaning workspace
[2021-03-25T02:23:09.531Z] Using shallow fetch with depth 3
[2021-03-25T02:23:09.531Z] Pruning obsolete local branches
[2021-03-25T02:23:09.432Z]  > git config remote.origin.url [email protected]:elastic/apm-agent-dotnet.git # timeout=10
[2021-03-25T02:23:09.439Z]  > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # timeout=10
[2021-03-25T02:23:09.448Z]  > git config remote.origin.url [email protected]:elastic/apm-agent-dotnet.git # timeout=10
[2021-03-25T02:23:09.469Z]  > git rev-parse --verify HEAD # timeout=10
[2021-03-25T02:23:09.487Z] No valid HEAD. Skipping the resetting
[2021-03-25T02:23:09.488Z]  > git clean -fdx # timeout=10
[2021-03-25T02:23:09.535Z] Fetching upstream changes from [email protected]:elastic/apm-agent-dotnet.git
[2021-03-25T02:23:09.535Z] using GIT_SSH to set credentials GitHub user @elasticmachine SSH key
[2021-03-25T02:23:09.550Z]  > git fetch --no-tags --progress --prune -- [email protected]:elastic/apm-agent-dotnet.git +refs/pull/1236/head:refs/remotes/origin/PR-1236 +refs/heads/master:refs/remotes/origin/master # timeout=15
[2021-03-25T02:23:10.196Z] Merging remotes/origin/master commit d1af211e98747b64f6704c214516cba33eb5237c into PR head commit 125b0cd6c9e6832a50144bf807400d5df256c22a
[2021-03-25T02:23:10.599Z] Merge succeeded, producing 9fd1e1c7a005ba3ef6aa42059793722993815800
[2021-03-25T02:23:10.599Z] Checking out Revision 9fd1e1c7a005ba3ef6aa42059793722993815800 (PR-1236)
[2021-03-25T02:23:10.779Z] Commit message: "Merge commit 'd1af211e98747b64f6704c214516cba33eb5237c' into HEAD"
[2021-03-25T02:23:10.792Z] First time build. Skipping changelog.
[2021-03-25T02:23:10.792Z] Cleaning workspace
[2021-03-25T02:23:10.200Z]  > git config core.sparsecheckout # timeout=10
[2021-03-25T02:23:10.205Z]  > git checkout -f 125b0cd6c9e6832a50144bf807400d5df256c22a # timeout=15
[2021-03-25T02:23:10.341Z]  > git remote # timeout=10
[2021-03-25T02:23:10.344Z]  > git config --get remote.origin.url # timeout=10
[2021-03-25T02:23:10.348Z] using GIT_SSH to set credentials GitHub user @elasticmachine SSH key
[2021-03-25T02:23:10.351Z]  > git merge d1af211e98747b64f6704c214516cba33eb5237c # timeout=10
[2021-03-25T02:23:10.590Z]  > git rev-parse HEAD^{commit} # timeout=10
[2021-03-25T02:23:10.603Z]  > git config core.sparsecheckout # timeout=10
[2021-03-25T02:23:10.614Z]  > git checkout -f 9fd1e1c7a005ba3ef6aa42059793722993815800 # timeout=15
[2021-03-25T02:23:10.782Z]  > git rev-list --no-walk e295d95e49785fa1d298f6c0b80548b65fe89fbc # timeout=10
[2021-03-25T02:23:10.795Z]  > git rev-parse --verify HEAD # timeout=10
[2021-03-25T02:23:10.803Z] Resetting working tree
[2021-03-25T02:23:10.804Z]  > git reset --hard # timeout=10
[2021-03-25T02:23:10.955Z]  > git clean -fdx # timeout=10
[2021-03-25T02:23:11.756Z] Masking supported pattern matches of $GIT_USERNAME or $GIT_PASSWORD
[2021-03-25T02:23:12.360Z] + git fetch https://****:****@github.com/elastic/apm-agent-dotnet.git +refs/pull/*/head:refs/remotes/origin/pr/*
[2021-03-25T02:23:14.370Z] Running in /var/lib/jenkins/workspace/net_apm-agent-dotnet-mbp_PR-1236/apm-agent-dotnet/.git
[2021-03-25T02:23:14.493Z] Archiving artifacts
[2021-03-25T02:23:15.432Z] + git rev-parse HEAD
[2021-03-25T02:23:15.783Z] + git rev-parse HEAD
[2021-03-25T02:23:16.104Z] + git rev-parse origin/pr/1236
[2021-03-25T02:23:16.186Z] [INFO] githubEnv: Found Git Build Cause: pr
[2021-03-25T02:23:16.651Z] Masking supported pattern matches of $GITHUB_TOKEN
[2021-03-25T02:23:17.576Z] [WARN] githubApiCall: The REST API call https://api.github.com/repos/elastic/apm-agent-dotnet/pulls/1236/reviews return 0 elements
[2021-03-25T02:23:17.631Z] [INFO] githubPrCheckApproved: Title: Allow access to MetricSet and improve memory allocation on MetricSample - User: glucaci - Author Association: CONTRIBUTOR
[2021-03-25T02:23:17.972Z] ERROR: githubPrCheckApproved: The PR is not allowed to run in the CI yet
[2021-03-25T02:23:17.972Z] ERROR: githubPrCheckApproved: The PR is not allowed to run in the CI yet. (Only users with write permissions can do so.)
[2021-03-25T02:23:18.036Z] [INFO] Let's stop build #4. The PR is not allowed to run in the CI yet
[2021-03-25T02:23:18.062Z] Sleeping for 5 sec
[2021-03-25T02:23:18.945Z] Stage "Parallel" skipped due to earlier failure(s)
[2021-03-25T02:23:19.034Z] Stage "Linux" skipped due to earlier failure(s)
[2021-03-25T02:23:19.035Z] Stage "Windows .NET Framework" skipped due to earlier failure(s)
[2021-03-25T02:23:19.035Z] Stage "Windows .NET Core" skipped due to earlier failure(s)
[2021-03-25T02:23:19.036Z] Stage "Integration Tests" skipped due to earlier failure(s)
[2021-03-25T02:23:19.037Z] Stage "Benchmarks" skipped due to earlier failure(s)
[2021-03-25T02:23:19.116Z] Failed in branch Integration Tests
[2021-03-25T02:23:19.117Z] Failed in branch Benchmarks
[2021-03-25T02:23:19.117Z] Stage "Linux" skipped due to earlier failure(s)
[2021-03-25T02:23:19.118Z] Stage "Windows .NET Framework" skipped due to earlier failure(s)
[2021-03-25T02:23:19.119Z] Stage "Windows .NET Core" skipped due to earlier failure(s)
[2021-03-25T02:23:19.242Z] Stage "Linux" skipped due to earlier failure(s)
[2021-03-25T02:23:19.243Z] Stage "Windows .NET Framework" skipped due to earlier failure(s)
[2021-03-25T02:23:19.244Z] Stage "Windows .NET Core" skipped due to earlier failure(s)
[2021-03-25T02:23:19.352Z] Failed in branch Linux
[2021-03-25T02:23:19.385Z] Stage "Windows .NET Framework" skipped due to earlier failure(s)
[2021-03-25T02:23:19.387Z] Stage "Windows .NET Core" skipped due to earlier failure(s)
[2021-03-25T02:23:19.495Z] Stage "Windows .NET Framework" skipped due to earlier failure(s)
[2021-03-25T02:23:19.496Z] Stage "Windows .NET Core" skipped due to earlier failure(s)
[2021-03-25T02:23:19.619Z] Failed in branch Windows .NET Framework
[2021-03-25T02:23:19.621Z] Failed in branch Windows .NET Core
[2021-03-25T02:23:19.794Z] Stage "Release to feedz.io" skipped due to earlier failure(s)
[2021-03-25T02:23:19.870Z] Stage "Release" skipped due to earlier failure(s)
[2021-03-25T02:23:19.902Z] Stage "Release" skipped due to earlier failure(s)
[2021-03-25T02:23:19.970Z] Stage "Release" skipped due to earlier failure(s)
[2021-03-25T02:23:20.109Z] Stage "AfterRelease" skipped due to earlier failure(s)
[2021-03-25T02:23:20.142Z] Stage "AfterRelease" skipped due to earlier failure(s)
[2021-03-25T02:23:20.699Z] Running on worker-1225339 in /var/lib/jenkins/workspace/net_apm-agent-dotnet-mbp_PR-1236
[2021-03-25T02:23:20.961Z] [INFO] getVaultSecret: Getting secrets
[2021-03-25T02:23:21.373Z] Masking supported pattern matches of $VAULT_ADDR or $VAULT_ROLE_ID or $VAULT_SECRET_ID
[2021-03-25T02:23:23.947Z] + chmod 755 generate-build-data.sh
[2021-03-25T02:23:23.947Z] + ./generate-build-data.sh https://apm-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/apm-agent-dotnet/apm-agent-dotnet-mbp/PR-1236/ https://apm-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/apm-agent-dotnet/apm-agent-dotnet-mbp/PR-1236/runs/4 ABORTED 178380
[2021-03-25T02:23:23.947Z] INFO: curl https://apm-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/apm-agent-dotnet/apm-agent-dotnet-mbp/PR-1236/runs/4/steps/?limit=10000 -o steps-info.json
[2021-03-25T02:23:24.648Z] INFO: curl https://apm-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/apm-agent-dotnet/apm-agent-dotnet-mbp/PR-1236/runs/4/tests/?status=FAILED -o tests-errors.json
[2021-03-25T02:23:24.648Z] Retry 1/3 exited 22, retrying in 1 seconds...
[2021-03-25T02:23:26.099Z] Retry 2/3 exited 22, retrying in 2 seconds...

@glucaci
Copy link
Contributor Author

glucaci commented Mar 22, 2021

If this is ok to merge I would like to start with tags support,
I saw that is supported on the server schema https://github.com/elastic/apm-server/blob/7cf56b584c9ff94153018afd1758f27737628873/docs/spec/metricsets/metricset.json#L47

@glucaci
Copy link
Contributor Author

glucaci commented Mar 22, 2021

@russcam can we merge this?
Thanks!

@gregkalapos
Copy link
Contributor

@glucaci let me try to give an overview on what we had in mind.

We'd like to have something similar across the Elastic APM Agents. One metrics library we wanted to have support for across agents is Prometheus: #1127 - at the moment we don't actively work on that in .NET.

The current metrics API was internal on purpose: we were not sure we'd like to have it public in its current form. If we make anything public, likely we'd prefer to have an internal sync with other Elastic APM agent devs to make sure that if we make this public then the API is somewhat similar across agents and we don't end up breaking it later.

So what I want to say with this: we may need to think about this a bit longer.

@glucaci
Copy link
Contributor Author

glucaci commented Mar 22, 2021

Thanks and I understand your point of view. I thought that this #322 (comment) was the final decision.

On the other hand is like a buggy API on IPayloadSender because any custom implementation of IMetricsSet will not work and this change would be a quick win for 1.x version.

In my opinion this will be a fix for anyone how wants to use custom metrics with 1.x and could be improved with a braking change on a 2.x version.

@russcam
Copy link
Contributor

russcam commented Mar 26, 2021

@glucaci thank you for your interest in improving the agent regarding metric collection!

The MetricSample change from class to readonly struct looks like a good one to make, but we would need to make it in a 2.x version since it would be a breaking change. With the change to MetricSet, I think this is something worth discussing more on #322 before committing to a PR, so I'm going to close this for now.

@russcam russcam closed this Mar 26, 2021
@glucaci
Copy link
Contributor Author

glucaci commented Mar 26, 2021

Hi @russcam, I'm not sure that readonly struct was a problem, I could revert it because indeed it's a braking change.

The main point of this PR is to make MetricSet public which is a must that IPayloadSender.QueueMetrics works. (at least in 1.x)

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.

4 participants